Review Size
The same reviewer finds real problems in a sixty-line change and waves through a nine-hundred-line one. Diff size predicts what review catches better than almost anything else about the reviewer.
The requirement, the obvious build, and why it breaks
Every lesson starts where the work starts: someone asked for something, and the first implementation that comes to mind survives until the requirement changes.
Why does the same person review a small change carefully and a large change not at all — and what do you do when the change genuinely is large?
A feature lands as one pull request: 47 files, 1,100 lines, a refactor, a schema migration and the feature itself. It is approved in six minutes with the comment "LGTM, nice work".
Ship the feature as one pull request so the reviewer sees the whole intent at once. Splitting it into six would make each piece meaningless out of context, and reviewing a fragment is worse than reviewing the whole.
It assumes the reviewer reads the whole thing. Past a few hundred lines they do not — they read what looks interesting and skim what looks routine, and the mistakes are in the routine parts, because the author skimmed those too.
- It assumes the reviewer reads the whole thing. Past a few hundred lines they do not — they read what looks interesting and skim what looks routine, and the mistakes are in the routine parts, because the author skimmed those too.
- The refactor and the feature are indistinguishable in the diff. Six hundred lines of moved code hide the four lines that changed behaviour, and no amount of care recovers them.
- A large PR is expensive to reject. By the time it arrives, a week of work exists, the deadline is closer, and a reviewer who wants a structural change is asking for a rewrite — so they do not ask (Review as Design Feedback — and Why It Arrives Too Late).
- Feedback arrives at the worst moment. Any comment on an early decision applies to code that everything else was built on top of, which makes the cheapest comments to make the most expensive to act on.
- It also delays every other change. A long-lived branch drifts from main, and the merge conflicts are resolved by the author alone, unreviewed, at the end (Long-Lived Branches in DevOps).
What limits the solution, and what must never stop being true
This domain leads with these two. A design that ignores its constraints is not a design, and an invariant nobody named is one nothing is protecting.
- The feature really is a week of work; it cannot be made small by wishing.
- The team ships to a single main branch and cannot leave half-finished behaviour visible to users.
- The schema migration must land before the code that reads the new column, or the deploy breaks (Expand and Contract).
- Review tooling shows a flat file list, so a reviewer has no way to see the intended reading order.
- Main is always deployable. No slice may leave the system in a state that cannot ship.
- A reviewer's approval must mean they formed a view. Approvals that do not mean that corrupt every other approval, because nobody can tell them apart afterwards.
Who owns what, and where the seams fall
Responsibilities decide boundaries; boundaries decide what an interface has to say.
- The author owns slicing, and owns the reading order — a PR that says "read these three files first, the rest is mechanical" is doing the reviewer's hardest job for them.
- The author owns separating behaviour-preserving from behaviour-changing work into different pull requests. This is the single highest-leverage habit in this lesson (What Refactoring Actually Is).
- The reviewer owns saying "this is too large for me to review properly" rather than approving it, which is socially expensive and is the whole discipline.
- The team owns the mechanisms that make small slices shippable — flags, expand-and-contract migrations, dark code paths — because without them slicing is impossible and asking for it is just pressure.
- The natural seam is behaviour: one pull request should change behaviour in one way, or change none at all. A pure refactor and a pure behaviour change are both easy to review; the mixture is not.
- The second seam is deployability. A slice boundary must fall where main is still shippable, which is what forces flags and expand-and-contract into this lesson (Feature Flags and What They Cost).
- The third is comprehension: a slice should be explicable in two sentences. If it cannot be, the boundary is in the wrong place, and the reviewer will have to reconstruct the intent from the code.
What a reviewer actually does at each size
It is more useful to describe behaviour than to state a rule. What changes with size is not how hard the reviewer tries; it is what they are physically doing — reading, skimming, or scrolling.
No numbers appear in this table on purpose. The band a change falls into depends on how dense it is, and a rule expressed in lines gets gamed by splitting at the line count instead of at the seam.
- The bottom two rows are why line count is the wrong rule: the same number means opposite things depending on density.
- The dangerous row is the third. It looks like review is happening — there are comments — and the comments are all in the interesting quarter.
- The fourth row is the one to make socially acceptable to refuse. Everything else in this lesson exists so that refusing is possible.
| Change size | What the reviewer actually does | What gets found | What gets missed |
|---|---|---|---|
| A handful of lines | Reads every line twice and thinks about the code around it | Logic errors, missing cases, a better name, an invariant the change routes around | Whether this was the right change at all — the context is not in a diff this small |
| A few dozen lines | Reads all of it, holds it in one head, compares it against the ticket | Nearly everything review is good at: behaviour against requirement, invariants, new failure paths | Consequences outside the diff, unless the author names them |
| A few hundred lines | Reads the parts that look interesting, skims the rest | Problems in the interesting parts | Everything in the parts that looked routine — which is where the mistakes are, because the author skimmed them too |
| A thousand and up | Scrolls, confirms CI is green, approves | Effectively nothing | Everything. This is not review, it is a countersignature, and it is indistinguishable from review in the audit log |
| Large but mechanical — a rename, a codemod, a lockfile | Correctly skips it | n/a — and skipping is the right call | The three hand-written lines someone slipped into a 4,000-line generated diff (Dependency Management) |
| Small but dense — a state machine, a pricing rule | Reads it several times and still may not be sure | Whatever the reviewer has domain context for | Anything requiring knowledge of the rule that is not written down anywhere (Ubiquitous Language) |
Splitting by behaviour, not by file
The instinct when told to make a pull request smaller is to split by file or by layer, which produces fragments nobody can evaluate. The seam that works is behaviour: does this change what the system does, or not?
A behaviour-preserving pull request is reviewed by reading the tests and confirming nothing in them moved. A behaviour-changing pull request is usually small once the preparation has already landed. Mixed together, neither is reviewable.
#412 "Add multi-currency support" 47 files, 1,100 lines - renames Money -> Amount everywhere (620 lines) - extracts CurrencyConverter from Pricing (180 lines) - adds currency column + backfill (90 lines) - changes checkout to convert at capture (14 lines) - updates 9 test files (196 lines) Approved in 6 minutes. The 14 lines that changed behaviour were never read.
#412 rename Money -> Amount 620 lines, no behaviour change
reviewed by: tests unchanged? yes. merge.
#413 extract CurrencyConverter 180 lines, no behaviour change
reviewed by: tests unchanged? yes. merge.
#414 expand schema + backfill 90 lines, no reader yet
reviewed by: is the backfill resumable? ([[data-migration]])
#415 convert at capture, flagged 14 lines + 40 lines of tests
reviewed by: reading all 54 lines, twice.The fourteen lines in #415 are the entire change in behaviour and the only place a currency bug can live. In the first shape they are 1.3% of the diff and arrive after the reviewer's attention is gone; in the second they are the whole pull request. Nothing about the work changed — only which lines the reviewer was looking at when they had attention left (What Refactoring Actually Is).
What slicing costs
Small pull requests are not free, and the case for them is not that they save time. They cost more total review time, more author overhead, and — if the mechanism is flags — a permanent addition to the system's state space.
The comparison worth making is between delivery shapes, and the honest position is that the size effect is well supported while the ranking of these three shapes is not.
| Option | Simplicity | Testability | Operational | Migration cost | Note |
|---|---|---|---|---|---|
| One pull request per feature | Simplest to plan and the only shape where a reviewer sees the whole intent at once. Everything downstream of that is worse: no real review, no partial revert, a long-lived branch, and a merge nobody checked. | ||||
| Sliced, behind a feature flag | Each slice is reviewable, each merge is revertible, and the migration lands separately from the code that depends on it. Paid for with a flag that has its own state space, its own tests and a cleanup obligation (Feature Flags and What They Cost). | ||||
| Stacked pull requests | Keeps each review small without adding a runtime flag, and preserves the reading order explicitly. Every rebase moves every PR above it in the stack, and tooling support is uneven enough that this is often more expensive than it looks. |
caveat These scores compare delivery shapes for one feature on one team; none of them is a defect rate and none of them was measured. What the empirical work supports is the narrow claim that review effectiveness falls as change size grows — it says nothing about which of these three mechanisms a given team should use to get small changes, and that choice is dominated by whether the team already has flags and stacked-PR tooling.
How to build it
Most important first.
- Split by behaviour first: everything that preserves behaviour goes in one pull request, everything that changes it in another. The refactor PR can be reviewed by reading the tests, and the behaviour PR is then usually tiny.
- Land the enabling changes ahead of time. Schema expansion, a new interface with a single existing implementation, an extracted seam — each is independently reviewable and independently safe (Seams).
- Put the behaviour change behind a flag so it can merge before it is visible, which converts "finish the feature" from a branch-length problem into a series of merges (Feature Flags and What They Cost).
- Write the reading order into the description. Which files carry the decision, which are mechanical, what you want scrutinised. Reviewers follow it, and it costs two minutes.
- Keep generated and mechanical changes in their own commits or their own pull requests — a lockfile update, a rename applied by the IDE, a formatting pass. Mixed in, they make the human part unreadable (What to Automate Out of Review).
- When the change genuinely cannot be sliced — a cross-cutting rename, a framework upgrade — say so, and review it differently: pair on it, or review the plan rather than the diff. Pretending to review it is the only unacceptable option.
What the next change costs
The field this whole domain exists for. A structure is only better if it makes the change after this one cheaper — and it is worth saying which changes it does not help.
- Under one-big-PR: the next change costs a week of branch life, a merge conflict resolution nobody reviews, and a review that does not happen. Crucially, the cost of the change after that is unchanged, because no design feedback was ever delivered.
- Under sliced-behind-a-flag: the next change costs several small reviews with real attention, plus a flag to remove later. The design feedback arrives on slice one, where acting on it costs a day instead of a week.
- What gets dramatically cheaper is reverting. A sixty-line merge can be backed out in a minute with a known blast radius; a 1,100-line merge cannot be reverted at all once anything has been built on it (Rollback: Only Useful If It Is Actually Safe in DevOps).
- What does not get cheaper: total review time goes up, not down. Six reviews cost more reviewer-minutes than one, and the argument for slicing is entirely about what those minutes buy, not about saving them.
- Small pull requests cost more total reviewer time and more author overhead, and they impose latency: a feature that could have merged once now waits six times.
- Flags buy sliceability with a permanent state-space cost and a cleanup burden that teams reliably underestimate. A codebase with forty stale flags has bought its small PRs at a real price.
- The reviewer's "this is too large" is socially expensive and lands on an author who has already done the work. Making it routine requires the team to back it, and where it is not backed the advice is unusable.
What can go wrong
- Slicing without a flag, so half-built behaviour is visible in production and the "slice" is actually an unfinished feature.
- Slicing by file rather than by behaviour, producing six pull requests none of which make sense alone — the reviewer now has all the cost of small PRs and none of the benefit.
- Author fatigue. Six PRs is six review cycles, six waits, six context switches; the honest response to "why did you make it one PR" is often "because six would have taken three weeks" (Review as a Gate in DevOps).
- The mitigation fails: a team that mandates a line limit gets pull requests split at exactly the limit, mid-thought, which is worse than either extreme.
- A large PR gets a "size" label and a second reviewer, and now two people are each assuming the other read it carefully.
- Slicing depends on flags, and flags have a cost of their own: a state space that grows with each one and a cleanup obligation nobody enjoys (Feature Flags and What They Cost).
- It depends on trunk-based habits. A team with long-lived release branches has structural pressure toward large merges regardless of individual discipline (Trunk-Based Development in DevOps).
- Stacked pull requests depend on tooling that most teams do not have, which is why the advice "just stack them" is cheaper to give than to follow.
- "So there should be a line limit." A limit produces changes split at the limit rather than at the seam, and a 400-line mechanical rename is far more reviewable than a 90-line change to a pricing rule. Size is a signal about attention, not a rule about lines.
- "Small PRs mean less design thinking." The opposite: slicing forces you to name what each step does independently, which is design work done before the code rather than after it (Slicing a Feature).
- "The reviewer should just try harder on big PRs." Attention is not a matter of effort. The reviewer who reads a thousand lines carefully takes four hours and finds the same things a fresh reviewer finds in the first two hundred.
- "This is a process rule." It is a claim about human attention. That claim happens to have better empirical support than most of what this domain teaches, which is a reason to take it seriously and not a reason to turn it into a gate.
- shotgun-surgery
Testing it, and how it ages
- Each slice must be independently green and independently deployable. If slice three requires slice four to pass its tests, the seam is wrong.
- For the pure-refactor slices, the test suite is the review: nothing in the assertions should change, and a diff that touches tests in a "no behaviour change" PR is the thing to look at hardest (Characterization Tests).
- Test the flag's off-path as well as its on-path, because until the flag is removed the off-path is what production is running (Feature Flags and What They Cost).
- As a team grows, the pressure toward large PRs grows with it — more coordination, more waiting, more incentive to batch. Slicing discipline that was natural at five people has to become explicit at fifty.
- As tooling improves — stacked PR support, better rename detection, semantic diffs — the size at which review degrades moves upward. It moves; it does not disappear.
- What eventually forces a change: an incident traced to a change nobody actually read, followed by the discovery that its approval looks exactly like every other approval in the history.
Where this applies
This domain's advice is contested more than most. These labels say what each claim is specific to — and where CONTESTED appears, the note gives the strongest form of the opposing view rather than a caricature.
- GENERALThe mechanism is human working memory rather than anything about a language or stack, so it holds everywhere; what differs is the size at which it bites, which is much lower for dense domain logic than for mechanical or generated changes.
- SIMPLIFIEDThe bands in the table below are qualitative and deliberately carry no numbers, because a number would imply a threshold that does not exist. Where the degradation begins depends on the density of the change: two hundred lines of a state machine exceed a reviewer long before two thousand lines of a mechanical rename.
- CONTESTEDThe relationship between review size and defect detection is one of the better-supported empirical claims in this domain — it has been observed repeatedly across industrial code review studies, which is more than can be said for most structural advice here. The genuine disagreement is about what follows from it. The opposing view holds that mandating small pull requests optimises a proxy: it produces a stream of individually reviewable changes whose *combined* effect nobody ever reviews, and that a reviewer who sees the whole feature at once is the only person positioned to notice that the design is wrong. That is a real cost of slicing, and it is the argument for reviewing the design separately from the diff (Design Review).
Where the depth lives
This domain teaches the codebase-level structure and hands the rest off.
- — Testing & Reliability Engineering — a sliced delivery depends on each slice being independently verifiable, which is a testing-strategy property before it is a review property.