Code Reviews That Improve Teams Instead of Slowing Them Down
The review is where standards are set, knowledge spreads, and morale is quietly made or lost.
7 min read
—Not yet rated
Loading…
The review is where standards are set, knowledge spreads, and morale is quietly made or lost.
Code review is the most consistently high-leverage practice in software engineering, and one of the most commonly done badly. When it works, defects are caught early, knowledge spreads across a team, and standards emerge without anyone writing a policy document.
When it goes wrong, the same ritual produces bottlenecks, resentment and a queue of pull requests that sit for three days before being approved unread.
The difference is rarely about technical rigour. It is about what reviewers look for, how quickly they respond, and how comments are phrased.
The most common review failure is spending the available attention on the least valuable layer. A reviewer who opens a pull request and immediately comments on a variable name has used their most alert minutes on the thing that matters least.
Working in that order means that when time runs out — and it always does — you have spent it on the parts a machine cannot check.
A review sitting for two days is not a careful review. It is a blocked colleague, a branch drifting further from main, and a context that has to be rebuilt from scratch when the comments finally arrive.
A reasonable norm is a first response within a few hours, on the same working day. That does not mean dropping everything the moment a notification arrives; it means treating review as scheduled work rather than as an interruption to be deferred indefinitely.
Review before starting new work, not after finishing it. Ten minutes at the start of your day and after lunch clears the queue and keeps everyone else moving. Reviews that wait for a gap in your own work never find one.
This is where reviews most often turn sour, and the fix is small. Comments should address the code and, where possible, be phrased as questions rather than verdicts.
'What happens if this list is empty?' invites the author to think, and occasionally reveals that they had a reason you did not see. 'This will crash on an empty list' asserts something you might be wrong about, and puts the author on the defensive either way.
| Instead of | Try | Why it works better |
|---|---|---|
| 'This is wrong.' | 'What happens when the input is empty?' | invites reasoning; you might be missing context |
| 'Why did you do it this way?' | 'Was there a reason not to use X here?' | reads as curiosity, not as an accusation |
| 'Bad naming.' | 'Would total_after_tax be clearer here?' | specific and actionable |
| A wall of 40 comments | the top 5, plus 'happy to pair on the rest' | 40 comments reads as a rejection |
| Silence, then approval | one substantive comment | silence looks like it was not read |
Label your comments so the author knows what is required. A simple convention works: blocking for things that must change, suggestion for things worth considering, and nit for minor preferences the author can ignore. Without labels, every comment reads as mandatory and reviews take twice as long.
Review quality falls sharply with change size. Under about 200 lines, reviewers read carefully and find real issues. Beyond 400 or 500, approval rates go up while defect detection goes down — people skim, then approve, because the alternative is an hour of unbudgeted work.
So the most effective thing an author can do for review quality is not writing better code. It is submitting smaller changes.
Split refactoring from behaviour change. Split an interface from its implementation. Ship a large feature as several merged increments behind a flag rather than one enormous branch — the same principle covered in our guide to Git workflows for teams.
For junior developers, review is often the primary way they learn how a team works. That makes the reviewer's tone consequential in a way it is not with peers.
Explaining why, not just what, is the whole difference. 'Use a transaction here' teaches nothing. 'Use a transaction here so a failure halfway through does not leave the order in a half-created state' teaches a principle they will apply next time without being told.
It works in the other direction too. Having juniors review senior code is one of the fastest ways for them to learn a codebase, and their questions frequently expose assumptions the rest of the team stopped noticing years ago.
A team of eight had pull requests waiting an average of two and a half days. Everyone was busy, and reviewing was what people did when they had a gap — which they never did. Branches were consequently long-lived, merges were painful, and the same conflicts recurred weekly.
They changed two things. Reviews were done at two fixed points in the day, before starting new work rather than after finishing it. And pull requests over 400 lines were sent back with a request to split them, no exceptions.
Median review time fell to about three hours. The unexpected result was that total throughput rose, because the team stopped losing days to conflicts and rework on branches that had drifted for a week.
Approval without reading is the worst outcome available, because it produces all the delay of a review and none of the benefit — while creating a documented impression that the change was checked. If you genuinely lack time, say so and hand it to someone else rather than rubber-stamping it.
Some disagreements do not belong in a comment thread. If a discussion is past four or five exchanges, or if it concerns architecture rather than this specific change, move it to a call or a shared document.
Written asynchronous discussion is excellent for specific, bounded points and poor at resolving fundamental disagreement, where it tends to escalate in tone while making little progress. Recognising which one you are in is a skill worth developing.
The goal of a review is not to produce the code you would have written. It is to confirm the code is correct, safe, and something the team can maintain.
— The standard worth holding to
Prioritise correctness and security over style. Respond the same day. Phrase comments as questions about the code and label what is blocking. Keep pull requests small — that single change does more for review quality than anything else. Explain the reasoning behind requests, and move long disagreements out of the thread.
Review is where a team's standards are actually set, whatever any document says. What gets challenged becomes the norm; what gets waved through becomes acceptable.
That makes it worth being deliberate about. A team with fast, kind, substantive reviews improves steadily without anyone running a formal quality initiative — which is a considerably better outcome than most initiatives achieve.
Tap a star to share what you thought.
No ratings yet
In order: correctness including edge cases, security and data safety, whether the design fits the codebase, readability, and finally style. Working in that order means your best attention goes to the things automated tools cannot check.
A first response on the same working day, ideally within a few hours. A pull request sitting for two days blocks a colleague, lets the branch drift from main, and forces everyone to rebuild context when the comments finally arrive.
Comment on the code rather than the author, ask questions instead of asserting conclusions, and explain the reasoning behind a request. Label comments as blocking, suggestion or nit so the author knows what actually needs to change.
Under a couple of hundred lines where possible. Review quality drops sharply with size: beyond about 500 lines, approvals go up while defects found go down, because reviewers skim rather than read carefully.
Yes. It is one of the fastest ways for them to learn a codebase, and their questions often surface assumptions the rest of the team stopped noticing. Pair it with the expectation that they ask rather than approve silently.
Respond with your reasoning rather than silently changing the code or silently resolving the comment. If the exchange goes past four or five messages, or concerns architecture rather than this change, move it to a conversation.
No — it is the worst outcome available. It creates all the delay of a review with none of the benefit, plus a record implying the change was checked. If you lack the time, say so and pass it to someone else.
Rarely. Adopt an automatic formatter and a linter so style is enforced by tooling, freeing reviewers to spend their attention on logic, security and design. Recurring style debates in reviews are a sign the tooling is missing.
Sign in to join the conversation.
Loading responses…
Have a story, idea, or something valuable to share? Join The Blog Story for free, publish your content, reach more readers, and earn a share of advertising revenue from eligible content.
Create quality content. Grow your audience. Grow your earning potential.