LUNAROPS · OPERATIONAL UPLINK 100% UPTIME 1,247d POSTS 893 JEFF.MOON@LUNAROPS.DEV UTC --:--:--

The Art of Code Review

13 min read Software Development
code-reviewcollaborationpull-requestsengineering-culturebest-practicesquality
Contents

The most common mistake teams make about code review is believing its purpose is to catch bugs. It catches some, but it is mediocre at it: a reviewer skimming a diff on a Tuesday afternoon will miss the race condition, the off-by-one, and the unhandled error that a test or a type checker would have caught deterministically. If bug-catching were the goal, the rational conclusion would be to automate review away. The reason code review survives — the reason every serious engineering organization mandates it — is that its real products are the things only humans can produce: shared understanding of the codebase, pressure on design before it ossifies, a propagating sense of the team’s standards, and the simple fact that after a review at least two people understand any given change. A team where only one person understands each subsystem is a team one resignation away from a crisis. Review is the mechanism that prevents that, and bug-catching is a bonus.

Getting review right is mostly about pointing humans at the problems humans are uniquely good at and ruthlessly delegating everything else to machines. A reviewer arguing about whether the brace goes on its own line, or whether the import order is alphabetical, is a reviewer not thinking about whether the caching strategy will fall over under load. The single highest-leverage thing a team can do for its review culture is to make those mechanical arguments impossible — automate them — so that human attention is reserved for design, logic, and maintainability, the things no linter will ever judge.

What human review is actually for

Strip away the bug-hunting myth and the genuine value of review sorts into a few categories, roughly in order of how much they justify a human’s time:

  • Design and approach. Is this the right solution to the right problem? A perfectly implemented wrong approach is the most expensive thing review can catch, because by the time it ships, ripping it out costs ten times what redirecting it at review would have. This is the review feedback that saves weeks, and it is invisible to every automated tool.
  • Readability and maintainability. Will the engineer who opens this file in eighteen months — quite possibly the author, having forgotten everything — understand it? Code is read far more than it is written, and the reviewer is the first proxy for that future reader. “I had to read this three times” is legitimate, important feedback.
  • Knowledge transfer. The reviewer learns how this part of the system works; the author learns from the reviewer’s questions. Review is how understanding diffuses through a team instead of pooling in individuals.
  • Consistency. Not formatting — a machine does that — but conceptual consistency: does this follow the patterns the codebase already uses for errors, for pagination, for configuration, or does it invent a fourth way to do a solved thing?
  • Catching what tools cannot. Subtle logic errors, missing edge cases, security implications of business logic, and “this is technically correct but will confuse on-call at 3 a.m.” — the judgment calls.

Notice what is absent from the list: style, formatting, import order, and anything a linter, formatter, type checker, or test can verify deterministically. Those belong to continuous integration, not to a human. The discipline of pushing every mechanizable check into CI is what makes the human review worth attending to.

Reviewing well: comments that land

A review comment is an act of communication under unusually bad conditions. Text strips tone, the author has emotional investment in code they just wrote, and a comment that reads as neutral to you can read as an attack to someone three time zones away at the end of a long day. The reviewer who internalizes this writes very differently from one who does not, and the difference shows up directly in how fast and how willingly authors act on feedback.

The most reliable upgrade is to explain the why and offer a path. A bare verdict gives the author nothing to act on and invites a defensive standoff; a reasoned suggestion teaches and resolves in one move.

1
2
3
4
5

Weak:    "This is inefficient."

Strong:  "This does an array .find() inside the loop, so it's O(n²) — with the
          ~5k items we see in prod that's noticeable. A Map keyed by id would
          make the lookup O(1). Want me to sketch it?"

The strong version states the problem, quantifies the impact, names a concrete fix, and offers help. It is almost impossible to take personally because it is about the code’s behavior, not the author’s character. A few more habits that consistently work:

  • Ask, don’t decree, when you might be missing context. “What happens if items is empty here?” invites the author to either fix a real gap or explain the guard you missed. You are often the one missing something.
  • Distinguish blocking from non-blocking. Label opinions: a prefix convention like “nit:” (trivial, take it or leave it), “question:”, and “blocking:” tells the author instantly what actually needs to change versus what is a musing. Without this, authors either over-react to nitpicks or under-react to real problems.
  • Praise specifically. “Nice — extracting this made the retry logic readable” is not fluff; it tells the author which instincts to repeat and makes the critical comments land as honest rather than hostile.
  • Review the code, never the coder. “This function does too much” not “you wrote a function that does too much.” The grammatical distance is small and the emotional distance is large.
Comment style Effect on the author Effect on the codebase
Bare verdict (“this is bad”) Defensiveness, slow response Problem may get patched without understanding
Reasoned + suggested fix Learns, acts quickly Right fix, and the lesson generalizes
Question (“what if X?”) Engages, surfaces hidden context Either a real fix or a documented rationale
Unlabeled pile of nits Can’t tell what matters; demoralized Real issues drown in trivia

The author’s half: making your code reviewable

Review is a two-sided contract, and authors control most of the variables that decide whether it goes well. The largest of these, by a wide margin, is the size of the change. Research on review effectiveness is unusually consistent here: reviewer defect-detection collapses as diffs grow, and beyond a few hundred lines reviewers stop reviewing and start rubber-stamping because the cognitive load exceeds what anyone can hold. A 2,000-line pull request does not get four times the scrutiny of a 500-line one; it gets a “LGTM” and a prayer.

So the foundational author habit is to keep pull requests small — ideally under ~400 lines of substantive change, and scoped to one logical thing. This is hard, and it is hard for the same reason small refactoring steps are hard: it requires planning your work as a sequence of independently-shippable increments rather than one heroic branch. The payoff is identical to the small-steps discipline in refactoring: small changes are reviewed faster, reviewed better, merged sooner, and reverted more cleanly when wrong. When a change is genuinely large, splitting it — a pure refactor in one PR, the behavior change in the next — is itself a gift to the reviewer, because it lets them verify “this moved code without changing it” separately from “this changes what the code does.”

The rest of the author’s job is reducing the reviewer’s cost of understanding:

  • Self-review first. Read your own diff in the review tool before requesting anyone. You will catch the debug print, the commented-out block, the leftover TODO, and the section that needs a comment — and you will save the reviewer from spending their attention on things you could have caught yourself.
  • Write the description the reviewer needs. Not “fixes the thing” but: what problem, what approach, what alternatives you rejected and why, what you want eyes on, and how you tested it. The description is where you front-load the context that makes the diff legible.
  • Make the diff tell a story. Coherent commits, no unrelated drive-by changes mixed in. A formatting sweep tangled into a logic change forces the reviewer to separate them mentally; do it for them by keeping them apart.
1
2
3
4
5
6
7
8

A reviewable PR:

  Title:  Fix gift-card checkout 500 on zero-total carts
  Size:   ~80 lines, 3 files, one logical change
  Desc:   Problem: full gift card -> total 0 -> div-by-zero in discount calc.
          Fix:     guard zero total before computing the ratio.
          Tested:  added regression test (red on old code); manual repro gone.
          Eyes on: the rounding behavior at line 42 — unsure it matches finance.

That last line — “eyes on X” — is the author doing the reviewer’s triage for them, pointing scarce attention at the spot that actually deserves it.

Receiving feedback without ego

The hardest skill in review is not technical; it is keeping your sense of professional worth detached from code you wrote an hour ago. Feedback on your code is not feedback on you, and the engineers who internalize that improve fastest, because they extract the lesson from every comment instead of spending energy defending. The ones who can’t turn every review into a negotiation, and their reviewers eventually learn to stop pushing — which is the worst outcome, because it means they stop learning.

Practical postures that keep ego out of it: assume the reviewer is trying to help, because they almost always are; when you disagree, explain your reasoning rather than just resisting, because you might be right, the reviewer might be missing context, and surfacing it is how the team converges; be genuinely open to the better idea even when it means redoing work, because sunk cost is not a reason to ship the worse design; and say thank you, because a reviewer who feels their effort is valued reviews more carefully next time. When a disagreement is real and unresolved after a round or two, escalate it to a synchronous conversation — a five-minute call resolves what twelve comment threads will not, and it does so without the slow-motion friction of asynchronous argument.

Automate everything a machine can judge

The recurring theme deserves its own section because it is the structural fix that makes everything else possible. Every minute a human spends on something a machine could have caught is a minute stolen from design and logic, and worse, it trains authors to associate review with pedantry. The goal is a pull request that arrives at human eyes only after the machines have already passed it.

   PUSH / OPEN PR
        |
   [ CI gate — must be green before a human looks ]
        |--- formatter (no style debates, ever)
        |--- linter / static analysis
        |--- type checker
        |--- unit + integration tests
        |--- security & dependency scan (e.g. OWASP checks)
        |--- coverage on changed lines
        |
   green? --> HUMAN REVIEW: design, logic, readability, edge cases
        |
   approve --> merge

This division of labor is the heart of a healthy review culture. Machines handle the deterministic, tireless, objective checks — and they never get tired, never play favorites, and never start a flame war over tabs. Humans handle the judgment: is this the right design, will the next person understand it, what breaks under load, does this business logic have a security hole that no scanner models? Security in particular is a split responsibility — automated scanners catch known-vulnerable dependencies and common injection patterns (the kind catalogued in the OWASP API Security Top 10), while a human reviewer catches the authorization check that is missing from a new endpoint, which no scanner can infer. Build the gate so that by the time a person opens the diff, “does it build, lint, type-check, pass tests, and clear the scanners” is already answered yes, and the human can spend their whole attention on the questions that need a human.

Keeping review from becoming the bottleneck

Done badly, code review becomes the slowest step in the pipeline — PRs sit for days, authors context-switch away and lose the thread, and the team’s throughput collapses into a queue of stale branches. The failure is usually cultural, not technical, and the fixes are about norms:

  • Treat reviewing as real work, not an interruption. A team that prioritizes shipping its own code over reviewing others’ produces a pile of blocked PRs and a throughput death spiral. Reviewing a teammate’s change so they can merge is often higher-leverage than writing your own next feature.
  • Set a response-time expectation, not a depth expectation. “First response within a few hours” keeps things moving; it does not mean “approve in a few hours.” A quick “looking now, give me an hour” unblocks the author’s planning even before the real review lands.
  • Right-size the rigor. A one-line config change and a new authentication flow do not need the same ceremony. Mature teams flex review depth to risk rather than applying maximum process uniformly, which is what burns reviewers out and slows everything to the speed of the most paranoid path.
  • Small PRs are the master lever again. Almost every review-latency problem traces back to PRs too big to review in one sitting. Fix the size and the latency, the quality, and the reviewer fatigue all improve together.

Verdict

Code review’s value is widely misunderstood, and the misunderstanding makes teams do it badly. It is not primarily a bug net — machines catch deterministic defects faster, cheaper, and without ego — and a team that treats review as bug-hunting will both over-invest human attention in mechanical checks and under-invest it in the things only humans can do. The real products of review are shared understanding, design pressure applied before mistakes set, and a codebase that no single person owns alone. Aim humans at design, logic, readability, and the edge cases that need judgment; push every formatting, style, type, and known-vulnerability check into the CI gate so it is already green before a person looks.

The behaviors follow from that frame. As a reviewer: explain the why, suggest a fix, label what blocks versus what is a nit, and critique the code rather than the person. As an author: keep pull requests small enough to actually review, self-review first, and write the description that front-loads the context — then receive the feedback as information rather than as a verdict on your worth. If one habit carries more weight than all the others, it is small pull requests: they get reviewed faster, scrutinized harder, merged sooner, and reverted more cleanly, and they make every other good review practice possible. Approach the whole thing as collaboration toward better software, not as a gate one person guards against another, and review stops being friction and becomes the mechanism by which a team gets collectively better than any of its members.

Sources

Comments