The craft of code review — reviews that don't wreck the team.
Code reviews are one of the most powerful tools in engineering — and one of the most frequently abused. Done well, they catch bugs, raise the team's bar, and spread architectural knowledge. Done poorly, they create resentment, slow shipping to a crawl, and turn the senior engineer into a bottleneck. The difference isn't talent. It's discipline around a small set of patterns.
01What reviews are actually for
Code reviews serve four purposes, in this rough priority:
- Catch bugs before users see them. Fresh eyes spot things the author missed.
- Spread knowledge. Reviewers learn about new parts of the codebase. Authors learn the team's standards.
- Maintain architectural consistency. Prevents the codebase from drifting in 12 different directions.
- Mentor and develop. Junior engineers learn craft by having their code reviewed by seniors.
Reviews are not for:
- Style nits that an auto-formatter could fix
- Demonstrating that the reviewer is smarter than the author
- Re-litigating decisions made in design review
- Personal preference enforcement
Most bad reviews fail because reviewers confuse purpose. They use a code review to do design review, or style review, or ego-feeding. The result is reviews that take days and produce resentment instead of better code.
02Giving reviews — Rule 1: review the code, not the person
Wrong: "You forgot to handle the null case."
Right: "This branch doesn't handle the case where userId is null. Was that intentional?"
The second version is identical in technical content but doesn't put the author on the defensive. The first version blames the author; the second talks about the code. Over hundreds of reviews, the cumulative effect is enormous.
Use the passive voice or question form for issues. Save direct praise ("Nice solution to X") for the active voice — praise is one place where personal attribution helps.
03Rule 2 — Distinguish severity
Mix three levels of feedback, but label them clearly:
- Blocking: "This must change before merge." Bugs, security issues, broken logic.
- Should fix: "I'd really prefer this changes, but I won't block on it." Style inconsistencies in obvious places, naming improvements.
- Nit / FYI: "Optional. Take or leave." Personal preferences, minor optimizations, "just so you know."
Many teams use prefixes: BLOCKING:, SUGGEST:, NIT:. This single discipline reduces review tension by 80%. The author knows which comments must be addressed and which are optional.
04Rule 3 — Approve more aggressively than you think
The best teams approve fast. Not because they have lower standards, but because they understand the cost of slow review.
Approval doesn't mean "I personally would have written every line this way." It means "this is good enough to ship; the team's overall quality bar is maintained or improved by this change."
If you have 6 nits and 2 suggestions but no blockers, leave the comments and approve. Don't make the author come back asking "is there anything else?" The friction of round-tripping is enormous and produces zero quality improvement.
05Rule 4 — Limit review depth
Research shows reviewer effectiveness drops sharply after 200 lines and again after 400. Beyond that, you're not catching bugs — you're rubber-stamping or fatigue-rejecting.
If a PR is over 400 lines, the right feedback is often "This is too large to review effectively. Can we split this into N smaller PRs?" That's not pedantic; it's honest. You will not give a 1,500-line PR the attention it deserves.
Exception: pure refactors and mechanical changes (renames, formatting). These can be huge and still safe to review. Look at the first 50 lines carefully, then skim the rest.
06Receiving reviews — Rule 1: separate technical from emotional
Every review comment will trigger a small emotional response. That's normal. Notice it, separate it from the technical question, and respond to the technical question.
If you genuinely disagree, say so. "I think the current approach is better because X. I'm open to changing it if you have a counter-argument." This is a perfectly valid response. Reviewers aren't infallible. Good reviewers want pushback when they're wrong.
What's not OK: silently changing things you disagree with because you're afraid of the conversation. The change goes in, but the resentment compounds and surfaces later in worse ways.
07Rule 2 — Make small PRs
If your PR is over 400 lines and the change isn't a mechanical refactor, you've already lost the review. Reviewers will skim, miss bugs, leave drive-by comments. Worse, the round-trip time will be days because nobody has time to review 1,000 lines carefully.
Split big changes into a stack of small ones:
- PR 1: refactor the existing code to support the new feature (no behavior change)
- PR 2: add the new abstraction with tests but don't wire it in
- PR 3: wire it in, replace old code paths
- PR 4: clean up old code paths
Each PR is reviewable in 10 minutes. The reviewer's attention is on each one. The bugs that would have slipped through a 1,500-line PR get caught in PR 2 of 4.
08Rule 3 — Write a real PR description
The PR description is for the reviewer, not the merge log. It should answer:
- What does this change do? (One paragraph)
- Why? (What problem it solves)
- What did you consider and reject? (Shows thought)
- How should the reviewer test it? (Specific steps)
- What are you uncertain about? (Where you want the reviewer to focus)
A good description cuts review time in half. The reviewer doesn't have to reverse-engineer your intent. They review against your stated goal.
09Cultural patterns that work
Strong review cultures share these:
- SLA on first response. 4 business hours from PR open to first reviewer comment. After that, the author should be unblocked to merge or escalate.
- Approval ≠ infallibility. If a bug ships, blame goes to the team, not the reviewer. Otherwise reviewers become paranoid and over-review.
- Pair programming counts. If two engineers worked on something together, no separate review needed. The pair is its own review.
- "Approved with comments." Reviewer can approve while leaving non-blocking suggestions. Author addresses them at their discretion and merges.
- Senior engineers don't bottleneck reviews. The senior reviews architectural changes and high-risk PRs. Junior-to-junior reviews are encouraged for routine changes.
10Anti-patterns to spot in your team
The 30-comment review on a 100-line PR. Almost always means the reviewer is doing style review, not code review. Auto-format. Use a linter. The review should focus on logic.
The "but have you considered..." review. The reviewer suggests every alternative they can think of without endorsing any. Forces the author to re-justify decisions. If you have a strong opinion, state it. If you don't, approve.
The architectural rewrite in code review. The reviewer suggests refactoring everything. This is design review masquerading as code review. If the architecture needs rethinking, that's a separate conversation that happens before the PR, not inside it.
The unanswered review. A junior posts a PR, gets no review for 3 days. Worst pattern. Teams that allow this signal that the junior's work doesn't matter. SLA is the fix.
The retaliation review. Engineer A reviewed B harshly. B is now reviewing A's PR with extra scrutiny. Toxic and obvious to everyone except the participants. Managers need to spot this.
∞What good reviews build
Good review culture is one of the highest-leverage investments a team can make. It compounds in three directions: code quality improves, junior engineers level up faster, and the team's collective intelligence about the codebase grows.
Bad review culture compounds in the opposite direction. The codebase degrades, the seniors become resented bottlenecks, the juniors stop trying, and the best engineers leave first.
The patterns above aren't complicated. They take discipline more than skill. Teams that commit to them consistently ship better code with less friction. Teams that don't pay the tax forever.