What to Automate Out of Review
Formatters, linters, type checkers, tests and static analysis exist so that human attention is spent only on what is arguable. Every mechanical comment a person still makes is a missing rule.
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.
Which review comments should never have to be made by a person again — and which ones look automatable but are not?
A team classifies six months of review comments. Sixty-one per cent are formatting, import order, unused variables and missing null checks. They want that number near zero without losing anything real.
Turn on a large, well-regarded rule set with everything enabled, set it to fail the build, and the mechanical comments disappear.
A maximal rule set contains rules the team does not agree with, and those get suppressed inline. Within a few months the suppressions are everywhere and no reader can tell which ones were considered (Comments).
- A maximal rule set contains rules the team does not agree with, and those get suppressed inline. Within a few months the suppressions are everywhere and no reader can tell which ones were considered (Comments).
- The rule sets mix categories.
no-unused-varsis mechanical and true;max-lines-per-functionis a contested design opinion converted into a threshold, and failing the build on the second one produces functions split at the limit rather than at a concept (Long Functions). - On an eight-year-old codebase, retroactive rules produce a ten-thousand-violation baseline that gets suppressed wholesale, and the rule then only applies to code nobody has touched.
- False positives compound. A scanner reporting two hundred advisories a week teaches the team to close the tab, and the one that mattered closes with it (Scanning, and Why a Finding Is Not a Risk in DevOps).
- And the real risk is the opposite of the intended effect: a green pipeline starts to feel like a reviewed change, so the human review that automation was supposed to free up quietly stops happening instead.
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 pipeline runs on every push and the team will not tolerate it taking more than a few minutes.
- The codebase is eight years old, so any rule turned on retroactively either fails ten thousand times or has to be scoped to changed files.
- Nobody will adopt a tool that produces false positives at any meaningful rate; one bad week trains everyone to ignore it permanently.
- The team has no appetite for a style debate, which is itself the strongest argument for a formatter with no options.
- If a rule can be decided from the diff alone, a person must never have to state it.
- A green pipeline must mean something. A check that is routinely overridden is worse than no check, because it consumes attention and provides no signal.
- Automation may lower the floor. It must not be allowed to raise the perceived ceiling — a green build is not a reviewed change.
Who owns what, and where the seams fall
Responsibilities decide boundaries; boundaries decide what an interface has to say.
- The formatter owns style completely, with no per-file overrides and ideally no configuration to argue about.
- The type checker and linter own mechanical correctness — nullability, unused code, unawaited promises, unreachable branches.
- Static analysis owns known-dangerous shapes: string-concatenated SQL, unvalidated redirects, secrets in source (Secrets in CI in DevOps).
- Architecture tests own the structural rules the team has actually agreed on — "nothing in domain/ imports from http/" — which is a real rule and not a proxy metric (Dependency Direction).
- Humans own everything that requires the requirement, the roadmap or the surrounding system: behaviour, design, security judgement, and whether the tests would fail (A Review Checklist Worth Reading).
- The dividing line is information: can this be decided from the diff alone? Format, types and known-bad shapes can. Behaviour against requirement cannot, and never will be, because the requirement is not in the repository.
- The second line is agreement. A rule should be automated once the team genuinely agrees on it, and not before — automating a live disagreement freezes one side of it into the build and removes the argument rather than settling it.
- The third is failure cost. Rules with near-zero false positives fail the build; rules that encode judgement warn. Mixing the two categories at the same severity is what destroys trust in the whole set.
Could a machine have said this?
Take any review comment and ask what information was needed to produce it. If everything required was inside the diff, a tool can decide it and a person should never say it again. If it required the ticket, the roadmap or knowledge of a system the diff does not mention, no tool has access to it.
The interesting cases are in the middle, where a tool can find the shape but not judge it — and those should warn rather than fail, because a warning invites a human decision and a failure replaces one.
- The first four rows should fail the build. The fifth should warn. The sixth is a person, and always will be.
- The third time you make the same comment by hand is the moment to consider a rule. Before that, the rule costs more than the comments (The Rule of Three).
- A rule that gets suppressed inline more than a handful of times is not a rule the team agreed on; it is one side of an argument that was never had.
What information did this comment require?
when "Import order", "trailing comma", "line too long"
cost A formatter on commit, enforced in CI. Zero remaining judgement — the choice is genuinely arbitrary, which is exactly why it must be settled by configuration and never by argument.
when "This can be null here", "unused import", "missing await"
cost Type checker and linter, at build-failure severity. The cost is false positives, which is why a rule that fires on correct code should be removed rather than suppressed at each site.
when "This concatenates user input into SQL"
cost Static analysis. It finds the shapes it has rules for and nothing else, so it lowers the floor without raising the ceiling — a novel authorization hole has no rule (SQL Injection and Parameterization in Security).
when "domain/ should not import from http/"
cost An architecture test — a real assertion about the dependency graph, cheap to write and unambiguous when it fails. Costs a rewrite whenever the structure legitimately changes (Dependency Direction).
when "This rule now lives in two places"
cost Warn at most. A tool can show textual duplication; only a person can say whether the two sites encode the same knowledge or merely look alike (Duplicate Knowledge).
when "The ticket says at period end; this cancels immediately"
cost Not automatable in any useful sense — the requirement is not in the repository. This is the review, and everything above exists to leave attention for it.
The order the gates should run
The sequence matters because each stage is there to prevent the next one from wasting attention. A human reached by a diff that has not been formatted, typed and tested is being asked to do work that a machine already declined to do.
Every stage below has a way of failing that converts it from a filter into noise, and each of those failures ends with the work landing back on the reviewer.
- 1Format, on commit
Eliminates the entire class of style comments before any human sees the diff.
fails by Reformatting a whole file in the same commit as a behaviour change, which hides four real lines inside six hundred mechanical ones (Review Size).
- 2Type check
Rejects shape errors that would otherwise need a careful reader to notice.
fails by A codebase where escape hatches are common enough that passing means very little.
- 3Lint, mechanical rules only
Encodes the mechanical decisions the team genuinely agrees on.
fails by Judgement rules at the same severity, which produces suppressions and then ambient suppression.
- 4Tests
Establishes that the behaviour the team wrote down still holds.
fails by Flakiness. Once red does not reliably mean broken, every check above and below is also downgraded to advisory (Flaky Tests in DevOps).
- 5Static analysis and dependency scanning
Finds known-dangerous code shapes and known-vulnerable versions.
fails by Volume. Two hundred advisories a week trains everyone to close the tab, and the one that mattered closes with it (Scanning, and Why a Finding Is Not a Risk in DevOps).
- 6Architecture tests
Asserts the structural rules that were actually agreed — dependency direction, package cycles, layer isolation.
fails by Being written during an active restructuring, so they encode the structure being replaced (Breaking Cycles).
- 7Human review
Behaviour against requirement, where the knowledge landed, security judgement, knowledge sharing.
fails by Being reached with the reviewer's attention already spent — which is precisely what every stage above exists to prevent (What Code Review Is For).
The stages are ordered by cost of failure, cheapest first, so that the expensive resource — a person — is the last thing consulted. A team that gets the order wrong, running a twenty-minute test suite before a one-second formatter, pays for it on every push.
When a design argument becomes a threshold
The characteristic failure of automated review is not too few rules. It is a design disagreement that someone resolved by converting their side of it into a number the build enforces.
The number then applies to code the original argument was never about, and the cheapest way to satisfy it is almost always to make the code worse in a way the tool cannot see.
looks like max-lines-per-function: 25, max-params: 3, max-depth: 3 and a cyclomatic-complexity ceiling, all at error severity — plus a scattering of inline suppressions on the functions where the team decided the rule was wrong.
suggests A real design argument that was settled by turning one position into a threshold rather than by convincing anyone. The tell is the suppressions: they mark exactly the cases where the team disagrees with its own rule, and nobody has taken that as evidence about the rule. The usual response to a violation is to split a coherent function into two incoherent halves, or to bundle three parameters into an Options bag with no meaning, both of which satisfy the number and worsen the code (Introduce Parameter Object).
fix Split the rule set by category. Mechanical rules fail the build; judgement rules warn and are reviewed periodically for whether anyone acts on them. Where the underlying concern is structural rather than aesthetic, replace the proxy metric with an architecture test that asserts the actual rule — "nothing in domain/ imports from http/" is checkable, unambiguous and says what it means, where a complexity ceiling is a number standing in for an argument nobody wrote down (Dependency Direction).
How to build it
Most important first.
- Start with a formatter that has no meaningful options, run it on commit, and reformat the entire codebase in one commit that touches nothing else. Style comments end that day, permanently.
- Enable mechanical rules as build failures, one at a time, fixing rather than suppressing. A rule that cannot be fixed across the codebase in an afternoon is a rule to scope to changed files instead.
- Keep judgement rules as warnings, always. Complexity ceilings, parameter counts and file lengths are useful smoke alarms and terrible gates (Long Parameter List).
- Prefer an architecture test to a metric whenever the underlying rule is structural. "No cycles between these packages" is checkable and means exactly what it says; "cyclomatic complexity under 10" is a proxy for something nobody stated (Circular Dependencies).
- Make the pipeline fast enough that nobody waits, because a slow pipeline pushes the mechanical work back onto reviewers and undoes the whole exercise (CI Is a Feedback System in DevOps).
- Write the rule the third time you make the same comment by hand — not the first. Below three, the rule costs more than the comments (The Rule of Three).
- Say out loud, repeatedly, that green is not reviewed. This is the mitigation for the failure mode this lesson creates.
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.
- Setting this up costs a few days once, plus one large mechanical commit that will conflict with every open branch — which is why it is done on a quiet Friday and announced first.
- After it: the next change costs a fast pipeline run and a human review that opens on the behaviour question instead of the import order. That is the entire return, and it is worth stating plainly, because it is a reallocation of attention rather than a saving of time.
- The next *rule* costs an afternoon of fixing existing violations, and that cost grows with the codebase — which is the argument for turning rules on early, when the fix is small.
- What does not get cheaper: none of this touches the design questions. A codebase can be perfectly linted and have its pricing rule in nine places, and no tool in this lesson has an opinion about that (Duplicate Knowledge).
- A formatter with no options means everyone loses some argument about style, permanently. That is the point, and it is still a real loss of expressiveness that some people feel keenly.
- Aggressive automation makes the pipeline a bottleneck and a gate; every new check is a new way for a legitimate change to be blocked at three in the afternoon on a Friday.
- Freeing up review attention does not guarantee it gets spent well. The honest position is that automation is necessary and not sufficient — it removes an excuse rather than creating a behaviour.
What can go wrong
- Suppression drift:
// eslint-disable-next-linebecomes ambient, and the rule now applies to nothing. - Gate proliferation: eleven required checks, three of which are unreliable, so the team acquires a culture of re-running until green — after which a genuine failure is also re-run until green (Required Checks in DevOps).
- Design rules as build failures, producing code contorted to satisfy thresholds. A 30-line function split into three 10-line functions with no names worth having is strictly worse than what it replaced (Extract Function).
- The mitigation fails: the team demotes everything to warnings, warnings are ignored by definition, and the whole apparatus becomes decoration.
- The success failure. Sixty-one per cent of comments disappear, review gets faster, and nobody notices that the freed attention went to closing the tab rather than to the design questions it was supposed to fund.
- Automated review depends on the pipeline being trusted. Flakiness converts every check into an advisory and pushes the work back to humans (Flaky Tests in DevOps).
- It depends on the rules being the team's and not a vendor's. Adopted wholesale, a rule set becomes an authority nobody can argue with, which is exactly the dynamic this domain warns about elsewhere (Changeability Is the Goal).
- Architecture tests depend on the structure being stable enough to assert. Writing them during an active restructuring produces tests that block the restructuring (Stable Boundaries).
- "So we can automate code review." You can automate the checkable subset. Behaviour against requirement, whether a boundary is in the right place, and whether the tests could fail all require information that is not in the repository.
- "Green build means it is fine to merge." Green means nothing mechanical is wrong. That is a floor, and the entire value of this lesson is spending the freed attention above it.
- "More rules means higher quality." More rules means more suppressions, unless each one is agreed and near-zero false positive. The count is not the variable (The Complexity Budget).
- "Complexity limits enforce good design." They enforce a number. The usual response is to split a coherent unit into incoherent pieces that satisfy the threshold, which is worse than the original and now passes (Over-Decomposition).
- long-parameter-list
Testing it, and how it ages
- Test the pipeline with a deliberately bad branch: misformatted, type-broken, with a lint violation and a hardcoded secret. Anything that reaches a human is a gap.
- Audit suppressions periodically. A count of inline disables per thousand lines, trending, says more about whether the rules are agreed than any discussion will.
- For architecture tests, verify they fail: introduce the forbidden import on a scratch branch and confirm the build goes red. An architecture test that has never failed may not be running (Dependency Cycles).
- The boundary moves in one direction. Every year, more of what needed a person becomes decidable by a tool, and the correct response each time is to remove that comment class from human review entirely rather than keeping it as a double-check.
- Automated reviewers that read the diff in context are the current edge of that movement, and they shift the line further than linters did — they can compare a diff against a linked ticket. They still cannot know what the team decided in a meeting, and treating their output as authoritative rather than as one more warning-level input is the predictable failure (The Model Is a Dependency).
- What eventually forces a rethink: the rule set is now eight years of accreted decisions, nobody remembers why half of them are on, and the honest move is to delete the ones nobody can justify — which is the same argument this domain makes about code (Speculative Generality).
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 information test — can this be decided from the diff alone — holds across every language; what differs is how much falls on the tool side, since a language with a strong type system moves whole comment categories into the compiler that a dynamic language leaves to review or to tests.
- LANGUAGE-SPECIFICIn Rust or Haskell a large class of review comments about lifetimes, exhaustiveness and error propagation is simply unavailable, because the compiler already refused. In Python or JavaScript the same concerns come back as review comments or as runtime failures, which shifts what the human review has to cover and makes gradual typing a review-productivity decision rather than only a correctness one.
- CONTESTEDThe strongest opposing view is that build-failing lint rules are a net negative: they encode one team's taste as law, they are routinely wrong, and the time spent satisfying them exceeds the time saved in review — with the added harm that engineers stop thinking about the underlying property and start thinking about the rule. Teams that have lived under a badly-chosen rule set hold this view with justification. The distinction that resolves most of it is mechanical rules versus judgement rules: almost nobody argues about
no-unused-vars, and almost everybody argues aboutmax-lines-per-function.
Where the depth lives
This domain teaches the codebase-level structure and hands the rest off.
- — Testing & Reliability Engineering — the trustworthiness of the test stage is what decides whether any of the gates above mean anything, and flakiness is the failure that degrades the whole pipeline at once.
- — Programming Languages & Runtime Internals — how much of this list a compiler can take over is a language-design question, and the answer has moved a long way in the last decade.