DEV Community

Cover image for I stopped reviewing my own code. Here's what had to be true first.
Isamu Arimoto
Isamu Arimoto

Posted on

I stopped reviewing my own code. Here's what had to be true first.

Strict lint rules replace human review

Most days now, I merge pull requests without reading the diff.

That sentence used to describe someone I would not have hired. So let me be precise about what changed, because it isn't confidence and it isn't recklessness. It's that I moved the things review was catching to somewhere that catches them earlier.

Here's the honest version of how that happened.

The problem was arithmetic, not philosophy

I run several coding agents in parallel. That produces more diff per day than I can read. Not "more than I feel like reading" — genuinely more than fits in a working day.

When that happens you have exactly two options:

  1. Generate less, so it fits what you can read.
  2. Make it safe to not read.

I picked the second one. Not because I'm brave, but because option 1 means throwing away the reason I set this up.

The uncomfortable part: option 2 is not a mindset. It's a list of specific things that have to be true. Here's mine.

1. The rules live in a file, not in review comments

Every code review I've ever done, the majority of my comments were mechanical. This function is too long. This nesting is too deep. Why is this any?

Machines can say all of that. So I made them say it, as errors:

"max-lines-per-function": ["error", { max: 60, skipBlankLines: true }],
complexity: ["error", 20],
"max-depth": ["error", 4],
"max-nested-callbacks": ["error", 4],
Enter fullscreen mode Exit fullscreen mode

Plus eslint-plugin-sonarjs with cognitive-complexity as an error, and @typescript-eslint's strict preset — any banned, non-null assertions banned.

Nothing here is novel. What's different is the next part.

2. The rules are stricter than a human team would tolerate

This is the part I find genuinely interesting.

If you put those thresholds on a human team, you get a PR relaxing them within a week. Not because engineers are lazy — because "this function is 63 lines and splitting it makes it worse" is sometimes true, and arguing about it every time is exhausting.

Lint strictness has always been a trade-off between machine correctness and human patience. And loosening the rules was never really a technical decision. It was a social one.

An agent doesn't get annoyed. It reads the rule, splits the function, moves on. It has no opinion about being told to do it again tomorrow.

So the social cost went to zero, and once that happens the trade-off only tips one way. I turned everything up until it hurt, and nobody complained, because nobody was there to complain.

3. Exceptions live in the config with a reason, never inline

The moment rules get strict, real exceptions appear. If you allow // eslint-disable-next-line, the rules are dead within a month — that comment is invisible in review and permanent in practice.

So exceptions go in the config file, one line per reason:

files: [
  "src/components/Sidebar.vue",       // @keyframes — the "thinking" spinner ring
  "src/components/GuiPanel.vue",      // `.frame + .frame` sibling-combinator spacing
  "src/components/FilesOverlay.vue",  // :deep into CodeMirror's injected root
],
rules: { "vue/no-restricted-block": "off" },
Enter fullscreen mode Exit fullscreen mode

Every entry says why. And the comment above the block says: delete the entry when the reason goes away.

The difference is visibility. An inline disable is invisible. A growing allowlist in a config file is a thing you can look at and be embarrassed by.

4. The rules themselves are pure functions, and they're tested

This is the part most setups skip.

If a rule decides something — which file extension goes where, how a path is normalised, what counts as a valid session id — that decision is code, and code that is only exercised through a UI is code nobody tests.

So decisions get pulled out into pure functions in a shared module, and those functions get tested against the awkward cases: empty, null, boundary, wrong-cased, wrong-platform. That project currently has a few thousand test cases, and the vast majority are testing small pure functions rather than flows.

The point is not the count. The point is: when the rule is a pure function, the rule can be tested. When it's embedded in a component, it can only be reviewed. And I stopped reviewing.

5. CI runs on the OS your users have, not the one you have

Our whole team is on macOS. Our users are not.

So CI runs Linux and macOS on every PR, and Windows on a nightly schedule (it's slow, and daily is enough to catch drift).

This one paid for itself immediately, and not in the way I expected. It's the only environment where I can reproduce a Windows bug report at all. Before, a Windows issue meant asking the reporter to test my guesses. Now I push a branch.

We also write Windows-specific test cases deliberately — path separators, realpathSync behaviour, fs.watch differences — so that fixing one doesn't quietly break the others.

6. Something else reads the code — and it isn't the thing that wrote it

Claude Code writes. Then Codex reviews, and CodeRabbit reviews, and the loop runs until they stop objecting.

The mechanism that matters here isn't "AI review is good." It's that the writer and the reader are different models. A model reviewing its own output shares its own blind spots. Two different ones don't, mostly.

This is the closest thing to a replacement for what I stopped doing. It isn't as good as a careful human reviewer. It is much better than a tired human reviewer at 11pm on the fortieth PR of the day, which was the realistic alternative.

What I still look at

I want to be honest about the boundary, because "I don't review anything" would be a lie:

  • UI changes. Nothing in the list above can tell me a layout is ugly or a flow is confusing.
  • Anything I couldn't verify by running it. If the change is about behaviour under conditions CI doesn't reproduce, I go look.
  • Anything touching auth, permissions, or data loss. The blast radius is wrong for automation.

Everything else, I let through on green.

The honest cost

Three things I'd want to know if I were reading this skeptically:

It front-loads a lot of work. None of the six items above is free. If you set up two of them and stop, you have strictness without a safety net, which is worse than neither.

It only works if the stack is uniform. The reason my per-repo config files are nearly empty is that every project uses the same language, the same test runner, the same CI shape. If your repos disagree with each other, you'll be writing the same rules over and over. Fix that first; it's cheaper.

Speed is not correctness. Things do get shipped fast and fixed fast. What this setup buys is not "no bugs" — it's that the bugs that survive are the ones review wouldn't have caught either.

The thing I actually took away

I set all this up to save time, and it did. But that isn't the interesting part.

The interesting part is that lint strictness, test coverage, CI breadth — the whole category of "engineering discipline we know we should do but don't" — was never really blocked on knowing better. It was blocked on how much friction a human team will absorb before it starts negotiating.

That constraint just got removed. Not gradually. It's gone.

I don't think most of us have updated for that yet.

Everything above runs in the open: my global config
and the project it runs on, both MIT.
Copy whatever's useful.

Top comments (5)

Collapse
 
rulestack profile image
Rulestack

The social-cost-goes-to-zero observation is the sharpest part of this — lint strictness was always a negotiation with human patience, and agents just don't show up to that negotiation. In our setup the exceptions-in-config-with-reason rule quietly became the most useful audit trail in the repo, since it's the only place where 'the rule was wrong here' survives with its justification attached. Did any threshold turn out to be genuinely too strict once agents were the only ones subject to it?

Collapse
 
isamu profile image
Isamu Arimoto

"The only place where 'the rule was wrong here' survives with its justification attached" — that's a better description of what the allowlist is for than anything I wrote. We've been treating it as a place to park exceptions; you're describing it as the record of where the rule met reality. Same file, much better reason to keep it honest.

To your question: no threshold turned out to be too strict. What turned up instead were rules that were wrong in a way a number can't express.

Three kinds, in increasing order of how long they took to understand.

Two rules asking for opposite things. no-floating-promises wants void somePromise() to mark a deliberate fire-and-forget. sonarjs/void-use forbids the void operator. Both were on. There is no threshold that resolves that — you pick which failure you'd rather catch, and we picked the forgotten await. The config comment says so, because otherwise someone re-enables it in six months and the 66 voids start screaming.

Rules whose false positives are structural rather than incidental. sonarjs/function-return-type flagged three functions for returning a union — but the union is the contract ("tool" | { said } | null). reduce-initial-value wanted an initial value on two reduces that can't be empty, because a length === 0 guard sits on the line above and the rule can't see it. Adding one would be dead code that also changes the return type. Neither is "too strict"; both will keep producing the same finding forever, so they're off with the reason written down rather than suppressed one call site at a time.

A rule that was correct, reasoning from a type that was lying to it. This one took the longest. different-types-comparison flagged nine guards as always-false, including process.argv[2] === undefined. Following it would have deleted real checks. The rule wasn't wrong — noUncheckedIndexedAccess was off, so TypeScript was telling it process.argv[2] is string. We turned the flag on and the count went nine to four, and the remaining four were genuinely redundant. So the "too strict" rule was actually a correct rule downstream of a missing setting.

That last one changed how I read false positives now: suspect the upstream config before the rule.

The one thing I'd add to your audit-trail point — we also require the entry to say when it can be deleted, not just why it exists. Two of ours name an upstream issue number. It turns the allowlist from a graveyard into a queue.

Collapse
 
isamu profile image
Isamu Arimoto

Following up on my own answer, because I got the first example wrong and the way I got it wrong is a better illustration than the example was.

I said no-floating-promises and sonarjs/void-use contradict each other, and that we turned void-use off to resolve it. I went to write that up properly, and it isn't true. S3735 returns early when the operand is promise-like:

if (isVoid0(node) || isIIFE(node) || isPromiseLike(context, node)) return;
Enter fullscreen mode Exit fullscreen mode

So void somePromise() — the exact thing no-floating-promises asks for — is never reported. I checked with a three-line file: void work() where work returns a Promise, silent; void sync() where it doesn't, reported. Then I enabled the rule on the real repo, expecting 66 findings from our fire-and-forget markers. Three. All in one file, all void map.delete(...) in an arrow body typed : void — discarding a return value, nothing to do with promises.

So the rules never conflicted. What conflicted was my model of them.

Two things I'd take from that.

First, the config comment explaining the exception was itself wrong, and it had been sitting there being convincing for weeks. That's the failure mode of the audit-trail idea you described: the entry survives with its justification attached, and if the justification was wrong on the day it was written, it survives too, now wearing the authority of a decision someone deliberately made. Ours read "The two rules contradict each other; we chose the one that catches a forgotten await." Confident, specific, false. I'd trust that more than an inline disable, which is exactly the problem.

Second, this is my own answer to your original question, arriving late. You asked whether anything turned out to be too strict. Nothing did — but a rule turned out to have been turned off for a reason that didn't exist, which is worse, because nobody was ever going to re-examine it. Three findings stood between us and having that rule on.

The one that survives review is the third example from my earlier reply, and it survives for the opposite reason: different-types-comparison was correct and the type it was handed was wrong. That one I verified by turning noUncheckedIndexedAccess on and watching nine become four.

I've filed it against both repos. Article rewritten before publishing.

Collapse
 
nerd_snipe_dev profile image
Nerd Snipe

The pure function testing point is critical. Most teams test the output of a rule (e.g., does this code pass linting?) but rarely test the rule itself. What happens when the linter's internal state machine hits an edge case? I once saw a custom hook fail only when called in a specific component lifecycle order, requiring us to write integration tests for the build process, not just the components. That’s where the real complexity lives.

Collapse
 
isamu profile image
Isamu Arimoto

Yes — and I think the reason is the one you're pointing at.

Lint config composes. Presets inherit, later blocks override earlier ones, a parser gets silently replaced by the block below it. Every piece is simple; the combination isn't. So "I configured it" and "it is checking that code" are two separate claims, and only one of them is easy to verify.

One from this week. Our as ban is set to error — eslint --print-config <file> confirms it, severity 2, assertionStyle "never". That same file has a cast sitting in its Vue template. Running eslint on it reports nothing: templates are outside the AST typescript-eslint walks, so a rule configured as an error reports zero. I found it by grep, not by CI.

Your layering is the right frame, and I'd push it one level up — unit, integration, e2e, and then the same question about the checkers themselves. Not "is the rule in the config file" but "does it actually fail when it should".

What changed for me is that this became affordable. Maintaining tests used to be the expensive part, so verifying your own tooling never made the cut; it was always the thing you'd get to later. That cost dropped. The excuse went with it.

For custom rules there's a direct answer — RuleTester pins valid and invalid samples. It's the config-and-pipeline layer that stays thin, and that's the layer where our typecheck turned out never to have covered the server at all.