skip to content

Reviews on your team are approved within minutes, yet a silent data-corruption defect shipped. How do you make review effective?

level: seniorimportance: should knowfreq 54%

answer

  1. Approval is not evidence of reading
  2. Compare size against elapsed time
  3. Silent means no oracle exists
  4. Size and delay feed each other
  5. Aim attention at stored-data paths

basics

~20 s

Fast approval is a symptom, not a success. Measure change size against reading time, find where the defect could have been caught, then shrink changes and aim review at the risky surfaces — rather than asking reviewers to try harder.

solid answer

~50 s

First establish whether anyone is reading. Compare change size against time-to-approval: if a four-hundred-line change is approved in eleven minutes, the implied reading rate is not achievable, so the process is ceremonial and no amount of exhortation will change it. Then work backwards from the escape: for a silent corruption, ask what would have revealed it — usually not a sharper reader but a missing oracle, since nothing failed and nobody complained. The durable fixes are structural. Shrink the unit under review so reading it is possible within the time people actually have; require the author to self-review and annotate before asking anyone else; route changes that write or migrate stored data to a second reader with a narrow high-risk checklist; and make "which assertion would fail if this were wrong?" a standing review question. Counting comments as a target only manufactures cosmetic remarks.

code

pseudocode · 12 lines
pseudocode
function orderTotalCents(lines, taxRate):
    subtotal = 0
    for line in lines:
        subtotal = subtotal + line.priceCents * line.quantity

    # integer arithmetic: the fractional cent is discarded, never rounded
    tax = (subtotal * taxRate) / 100

    return subtotal + tax

# existing test asserts only that a total came back
assert orderTotalCents(cart, 7) > 0

go deeper

for a junior

Recall that an approval is not evidence anyone read the change, and that a passing suite only checks the cases somebody thought to assert. Being able to ask what would have failed if the code were wrong already puts you ahead.

for a middle

Explain the mechanics: why large changes get read less per line rather than more slowly, how size and turnaround reinforce each other, and why a silently wrong result is a missing assertion rather than a missing reader.

for a senior

Demonstrate the diagnosis from evidence — implied reading rate, where the defect could have been caught, which surfaces deserve concentrated attention — and propose structural fixes while ruling out the effort-based ones explicitly.

for a principal

Own the tradeoff between review cost and escape cost across teams. Be ready to argue how much of the organisation's change flow warrants concentrated attention, why uniform mandates degrade into ceremony, and how you avoid creating targets that manufacture activity.

## Read the symptom correctly Fast approval and an escaped defect together are strong evidence of ceremonial review: the process runs, produces an approval, and performs no detection. That is a different problem from slow or contentious review, and the fixes are different too. Before proposing anything, establish the fact. Take a concrete case. A four-person team owns the checkout of an online bookstore. Over the last quarter they merged 31 changes a week, median size 412 changed lines, median time from a change being opened to being approved 11 minutes, and a mean of 0.3 review remarks per change. A defect in the order-total calculation truncated a fractional tax amount instead of rounding it, so 1.7% of orders were stored with a total a few cents below what the customer was actually charged; it ran for 19 days before finance reconciliation surfaced the gap. The numbers settle the diagnosis without needing anyone's opinion. 412 lines in 11 minutes is roughly 37 lines a minute, sustained, including navigating between files. Nobody reads unfamiliar code that fast. The approvals are real; the reading is not. ## Why silence made it worse Silent corruption is the hard case for every detection mechanism a team has. No exception is raised, no page fires, no user complains, and the automated suite is green because the assertion that would have failed was never written — the test asserted a total was produced, not which total. Review is one of the few instruments that can catch this class at all, because it is the only one that compares code against *intent* rather than against an existing expectation. That is precisely what a ceremonial review does not do. So the review question that targets this class is not "is this code correct?" but **"which assertion would fail if this were wrong?"** Applied to the truncation, it exposes the gap immediately: no test pins the cent-level result of a fractional tax, so the reviewer is being asked to verify arithmetic by eye with no safety net. That finding converts into a test, which is durable, rather than into a reviewer's promise to be more careful, which is not. ## Fix the structure, not the effort **Shrink the unit under review.** Change size is the variable that dominates almost everything else about review. A large change is not reviewed proportionally more slowly; past a threshold it is reviewed *less* per line, because a reader cannot hold that much context and quietly switches to skimming. Splitting the same work into several changes — by taking a preparatory refactor first, or by introducing a new path alongside the old one and switching over separately — keeps every unit inside what a person can genuinely read. Widely-quoted guidance suggests a few hundred lines in one sitting of under an hour as an upper bound; that figure comes from a small number of industry studies and older inspection work, so treat it as an order of magnitude rather than a threshold to enforce. **Break the size-latency loop.** Size and delay reinforce each other. A large change is daunting, so it waits; while it waits the author starts more work on top of it; when it is finally read it is larger still and the reviewer is further behind. Teams caught in that loop cannot fix it by promising faster turnaround, because the size is what makes the turnaround slow. Shrinking the unit is what makes prompt reading feasible. **Make the author do the first pass.** Requiring a self-review before anyone else is asked — the author reading their own change as a stranger, and annotating the parts that are non-obvious or that they are least sure of — is cheap and removes a surprising share of findings before a second person spends time. It also gives the reviewer a map, which raises the reading rate honestly rather than by skipping. **Aim attention by risk.** Not every change deserves the same scrutiny, and pretending otherwise is how uniform inattention sets in. On a four-person team, route changes that write, migrate or reconcile stored data — and anything touching money — to a second reader with a short supplementary checklist, and let low-risk changes take a single quick read. Concentrating the available attention where an escape is expensive beats spreading it evenly and thinly. **Watch for the measurement trap.** Do not turn the diagnostic signals into targets. Remark counts and review durations are useful for spotting a ceremonial process; once a team is judged on them, remarks appear on cue and they are cosmetic. Judge the change instead: where defects are being found, and whether escapes are trending down. ## What to say out loud A strong answer states the diagnosis from evidence, names the class of defect and why review is the instrument that could catch it, and then proposes structural changes with the effort ones excluded explicitly — no team ever fixed ceremonial review by asking people to concentrate harder.

  • The team says they cannot split their changes because the work is inherently large. How do you respond?
    Almost no change is atomic. Most large ones decompose into a preparatory restructuring that changes no behaviour, then a narrow behavioural change, then a cleanup — or into introducing a new path beside the old one and switching over as a separate change. Splitting also needs somewhere to land: if a change can only be merged when the whole feature is finished, the constraint is the delivery model, not the code, and that is what to address.
  • How would you know six weeks later whether the changes worked?
    Look at where defects are being found rather than at review activity. The signals worth watching are the size distribution of merged changes, whether findings are appearing during review at all, and whether defects of the same class are still reaching production. Review remark counts are the wrong measure: they respond instantly to being watched, and what appears is cosmetic.
  • Would adding a second required approver have prevented this?
    Not on its own, and it can make things worse. Adding approvers to a process nobody is actually reading spreads responsibility thinner — each reader assumes the other looked closely — while adding delay that pushes the team back toward larger batches. A second reader helps when it is targeted at a narrow high-risk category with an explicit question to answer, not when it is applied uniformly.

saying these in an interview costs you the question

  • Treats fast approval as evidence of a healthy process
  • Proposes telling reviewers to be more careful
  • Adds required approvers to a process nobody reads
  • Assumes a green suite means the behaviour was verified
  • Sets a minimum comment count as a target
  • Blames the individual reviewer instead of the change size

context