Code review antipatterns
a day ago
- Code review can be used constructively to improve code and skills, but also obstructively with dark-side antipatterns.
- The 'Death of a Thousand Round Trips' involves repeatedly nitpicking to delay the patch, often across time zones.
- Hostage-taking: demanding unrelated work in exchange for approving a critical patch.
- Review ping-pong: two reviewers make incompatible demands, bouncing the developer back and forth.
- The Guessing Game: vague criticism of a solution without suggesting alternatives, forcing the developer to guess.
- Small nits first, then dropping a major rewrite: making the developer waste effort on trivial fixes before a fundamental change.
- Design wholesale: demanding justification for the entire project design during a minor patch review.
- One big patch vs. many small patches: criticizing whichever approach the developer chose.
- Arbitrary inconsistency: objecting to a pattern previously accepted, forcing the developer to update old code.
- The article is a joke; the real message is to avoid these behaviors, use review authority responsibly, and be constructive.
- In peer review, authority is temporary and should be used only to improve the patch, not for personal agendas.
- In gatekeeping (e.g., unsolicited patches), criticism can be legitimate but should be clear and not passive-aggressive.