Code review that helps: approve what improves the code, save the nitpicks for later
A practical code review standard for reviewers and authors: improve code health steadily, separate facts from taste, label nits and settle disagreements.
Anyone who has waited three days for a review knows how it feels. The change was fine. The comments were about naming, about the way a loop was written, about what the reviewer would have done. Meanwhile the work sat still.
Code review is one of the most valuable habits a team has, and one of the easiest to do badly. A clear standard helps both sides.
One question to anchor on
Google publishes its engineering practices, and the standard for code review is short. The goal is to keep the overall health of the code improving over time. Reviewers also have to let developers make progress, because a team that cannot ship cannot improve anything.
The rule that ties these together: approve a change once it definitely improves the overall code health of the system, even if it is not perfect. Not "is it how I would do it", but "is the codebase better with this than without it?"
That does not mean anything goes. The guide says plainly that nothing justifies submitting a change that clearly makes code health worse, apart from emergencies. And a reviewer can still decline a change for other reasons, such as a feature the team does not want.
Code is never perfect
There is a line in the guide worth remembering: there is no such thing as perfect code, only better code. If you demand that each author polish every small detail, reviews stretch out, authors get frustrated and the changes get bigger as they wait.
Two habits help:
- Label small preferences. The guide suggests prefixing minor points with "Nit:" so the author knows they can ignore them. A comment that is a nit and looks like a blocker costs the author an hour.
- Mark teaching comments as optional. Sharing a technique is welcome, but if it is not essential to this change, say so.
Nit: this could be a map lookup, but the loop is fine for now.
Blocking: this query runs once per row; with the real table
size that will time out. Could we batch it?
Two comments, two clear weights. The author knows what must change before merging and what is a free suggestion.
Facts before preferences
Disagreements get easier when you agree on what counts as evidence. The guide gives an order:
- Technical facts and data win over opinions and personal taste.
- The team's style guide is the authority on style. If it does not cover something, it is a matter of preference, and the existing code's style is the default. With no existing style, the author's choice stands.
- Design decisions rest on engineering principles, not on taste. If the author shows that several approaches are equally valid, the reviewer should accept the author's choice.
The effect is that "I would have written it differently" is not enough to hold up a change. "This will fail when the list is empty" is. If your team records why it made earlier choices, for instance in architecture decision records, a reviewer can point to the record instead of re-arguing the same point.
What authors can do
Review is a two-way street. Authors make the reviewer's job easier, and approval faster:
- Keep changes small. One concern per change. A reviewer can hold a 100-line diff in their head; a 1,500-line one gets skimmed.
- Write a description. Say what changed, why, and what you did not do. Point to where the risk is.
- Review your own diff first. You will catch leftover debug lines and unclear names before anyone else sees them.
- Reply to every comment. "Done", "Disagree, because..." or a question are all fine. Silence is not.
- Assume good intent. Comments are about the code, not about you.
What to look for
A useful order of attention, from the most important down:
- Does the change do what it claims, including the awkward cases (empty input, retries, concurrent access)?
- Is the design sensible for where it lives in the codebase?
- Is it tested, or at least checked in some way the team trusts?
- Will the next person understand it?
- Only then: naming, formatting and small style points, ideally caught by a formatter or linter rather than a human.
Automate whatever a tool can decide. Every comment about whitespace is a comment not spent on the part that needs a thinking person.
When you disagree
Sometimes two reasonable people do not agree. The guide describes a path:
- Try to reach consensus using the standard above.
- If that is slow, talk face to face or on a call. Write the outcome down as a comment on the change, so the reasoning is not lost.
- If it is still stuck, escalate: a wider team discussion, a technical lead, a maintainer or an engineering manager.
The sentence to keep in mind is that a change should not stall because the author and reviewer disagree. Escalating is not a failure; waiting indefinitely is.
Tip
Track how long changes wait for a first review. If the number is days, the process is the problem, not the people. Agree on a target such as one working day and see what gets in the way.
Review is also learning
Done well, review spreads knowledge. Authors learn the codebase's habits; reviewers learn what is changing. When something goes wrong later, the same open, blame-free attitude applies, as described in blameless postmortems. Both practices rest on the idea that the goal is a better system, not a winner.
A starting point for your team
Write three lines on the wall of your engineering handbook: approve changes that clearly improve the code, label nits, and decide disputes with facts. Then try it for a month and see whether changes merge faster and whether people still feel the quality is there. Most teams find that it is.
