Code Review

Before this

• Branching & Merging — Branches are pointers, merges are graph operations — that's the whole trick.

Why this exists

A bug reaches production that a second pair of eyes would have caught in thirty seconds. Code review exists because the person who wrote the code is the worst-positioned person to spot its flaws — they already believe it works, because they just wrote it that way on purpose. A reviewer brings exactly the skepticism the author cannot supply themselves.

The mental model

Think of review as a second compiler pass, but one that checks intent instead of syntax. A compiler asks "does this parse?" A reviewer asks "does this do what the ticket actually needed, will it survive the next six months of changes, and would a stranger understand it at 2am during an incident?"

How it works

The mechanism

A pull request opens with a diff. The reviewer reads the description for intent, reads the diff for the actual change, and reads surrounding code for context the diff omits. They leave comments, the author responds or pushes fixes, and the cycle repeats until the reviewer approves — at which point the change merges, typically gated by CI passing.

What people get wrong

The goal of code review is to find as many issues as possible.

The goal is to ship correct, maintainable code with reasonable speed — a review that nitpicks fifteen style issues and misses a security bug has failed at its actual job. Optimising for comment count rather than risk means reviewers spend their limited attention on the cheapest-to-spot issues instead of the ones that would actually hurt in production.

Approving a pull request means you'd have written it exactly this way.

Approval means you're confident the change is correct and reasonable, not that it matches your personal preference in every detail — reviewers who withhold approval over pure style differences slow teams down for no safety benefit. Conflating correctness with preference is the single biggest cause of review threads that drag on for days over something that does not affect behaviour.

A large, thorough-looking review comment thread means the review was good.

Comment volume correlates weakly with defects caught and strongly with how contentious the topic was — a quiet review that catches one real bug beats a fifty-comment thread about naming. Teams that measure review quality by activity accidentally reward reviewers for bikeshedding rather than for finding the issues that matter.

When not to use it

The change is a one-line typo fix or a version bump with no logic change.

A lightweight rubber-stamp approval is appropriate — reserve deep scrutiny for changes that touch behaviour, not for changes that touch nothing meaningful.

A design decision needs to be made before any code is written.

Have that conversation in a design doc or a synchronous discussion before the pull request exists — code review is the wrong venue for deciding architecture, because rewriting an already-built feature is expensive.

Terms

Blocking comment

— A review comment that must be addressed before the change can merge, as opposed to an optional suggestion.

Nit

— A minor, non-blocking style or preference comment, conventionally marked as such so the author knows it's optional.

LGTM

— "Looks good to me" — shorthand for approval, sometimes qualified as LGTM with a minor comment addressed first.

Review latency

— The time between a pull request opening and receiving its first substantive review — a major driver of how fast a team ships.