Code Review

Definition

Another engineer examines a change before it is merged. Its stated purpose is defect detection; its larger practical value is knowledge distribution, design feedback and shared ownership.

What review is good and bad at

Good at: spotting design problems, naming and clarity issues, missing edge cases, and spreading understanding of a system. Also good at catching the change that is technically correct and architecturally wrong — which nothing automated will find.

Bad at: finding subtle logic bugs (tests are better), enforcing style (automate it), checking security exhaustively (scanners plus targeted review of sensitive areas), and verifying performance.

Anything a machine can check should be checked by a machine, so human attention goes to what only a human can assess.

The property that determines its value: latency

Review latency is a first-order delivery metric. A change waiting eight hours for review means the author has context-switched, and returning to it costs more than the review saved. A team where review takes a day has added a day to every change.

The remedies are cultural rather than technical: review as the first task of the day, an agreed response-time expectation, and small changes — a 40-line change is reviewed in five minutes and a 1,000-line change is approved without being read.

Change size is the strongest predictor of review quality. Beyond a few hundred lines, defect detection falls off sharply because attention does.

What makes review good

• Small changes. Everything follows from this.
• A stated purpose per review — is this a design check, a correctness check, or a knowledge-sharing read?
• Distinguishing blocking from non-blocking comments. A reviewer's preference should be marked as such, not left ambiguous.
• Design discussed before implementation, so review does not become a late-stage architecture debate on 800 lines of finished work. That is the single most demoralising failure mode.
• Reviewing the tests as carefully as the code.

Failure scenarios

• Rubber-stamping large changes, so the process costs time and provides nothing.
• Style debates that automation should have settled.
• One bottleneck reviewer, whose absence stops the team.
• Review as a gate rather than a conversation, producing adversarial dynamics.
• Architecture raised for the first time in review, when the work is complete.

Interview question

"Review latency in your team averages a day. What would you change, and what would you measure?"