luismori.

Engineering Practices

Code Review Pull Requests Engineering Culture Developer Productivity

The Infinite Loop of PR Nits: Do We Really Need Perfect Pull Requests?

A practical guide for engineers on when nit comments help, when they become review noise, and how many are enough before a team should automate, approve, or move the discussion out of the PR.

15 min read

TL;DR

Nit comments are useful only when they stay small, explicit, and optional.

The best guidance I found is consistent:

  • A PR does not need to be perfect. It needs to improve the health of the codebase.
  • If a comment is polish, label it clearly as nit:, optional:, or suggestion (non-blocking):.
  • If the only remaining comments are nits, approve the PR.
  • If a nit repeats often, stop writing the comment and automate it with a formatter, linter, template, or style guide.
  • If a review has more than a couple of nits, batch them into one comment or ask whether the team is really reviewing the right thing.

My practical rule:

0 nits: best default
1-2 nits: okay if clearly optional
3+ nits: batch, automate, or convert into team guidance
only nits left: approve
new nits after fixes: usually stop

That is not a universal law. It is an operating rule that matches the spirit of Google, Microsoft, Conventional Comments, and several practitioner writeups: protect reviewer attention for correctness, design, tests, security, and maintainability.

What You Will Learn Here

  • What a nit comment is and why it creates so many PR loops.
  • Why “perfect PR” is the wrong review target.
  • What Google, Microsoft, Conventional Comments, and practitioner guidance say.
  • A practical answer to “how many nits are enough?”
  • How reviewers can separate blockers from optional polish.
  • How authors can respond without turning every preference into another commit.
  • How to automate repeated nits so humans review the important parts.

The PR Nit Loop

Most teams know this loop:

author opens PR
  |
  v
reviewer leaves useful comments + 8 tiny preferences
  |
  v
author fixes everything
  |
  v
reviewer rereads and finds 5 new tiny preferences
  |
  v
author fixes again
  |
  v
another reviewer adds different tiny preferences
  |
  v
PR waits, context decays, frustration grows

The problem is not that reviewers care about quality. They should.

The problem is that the review has lost its priority stack. A security bug, a missing test, a naming preference, and a blank line all arrive as the same GitHub notification. If everything looks mandatory, the author treats everything as mandatory. If everything is mandatory, the PR cannot finish.

That is how teams accidentally turn review into a search for the perfect diff.

What Is A Nit?

A nit is a small point of polish. Usually it is about naming, wording, formatting, readability, or a tiny simplification.

Good nit:

nit (non-blocking): I would rename `data` to `invoiceRows` so the next reader
knows what shape this has. Fine to keep as-is if you prefer this name.

Bad nit:

Rename this.

The first comment gives the author priority, intent, and permission to decide. The second comment creates ambiguity. Is this a blocker? A preference? A team convention? A request from a maintainer that must be obeyed?

Ambiguity is where PR loops grow.

The Perfect PR Trap

Google’s code review standard is the clearest source I found. It says reviewers should favor approving a change once it definitely improves the overall code health of the system, even if the change is not perfect.

That line matters because it separates two goals:

good review goal:
  improve the system over time

bad review goal:
  remove every possible imperfection from this PR

“Perfect” sounds responsible, but it often hides a cost:

  • The PR stays open longer.
  • The author keeps context-switching back to old work.
  • Reviewers spend attention on preferences instead of defects.
  • Important comments get buried under low-value comments.
  • The team learns that review is a negotiation tax, not a quality system.

The best review question is not:

Can I imagine one more improvement?

It is:

Is this change safe, understandable, tested enough, and better than what we had?

What Best Practices Say

Here is the research audit in plain language.

Google’s Engineering Practices say there is no such thing as perfect code, only better code. Reviewers should not require every tiny polish item before approval. If a comment is not very important, prefix it with Nit: so the author knows it is optional.

Google’s comment guidance also recommends labeling severity. Their examples include Nit:, Optional, and FYI, specifically to help authors understand what matters and avoid treating every comment as mandatory.

Microsoft’s Engineering Fundamentals Playbook says human reviewers should focus on business logic, tests, architecture, design, readability, maintainability, and common errors. It also says to prefix a “point of polish” with Nit: and to use automated styling tools like Black and Prettier for style.

Conventional Comments makes the same idea more systematic. It defines nitpick: as trivial, preference-based, and non-blocking by nature. It also supports decorations like (blocking) and (non-blocking) so review intent is visible.

Dan Lew’s “Stop Nitpicking in Code Reviews” is not an official standard, but it is a useful practitioner account. The strongest point: many nits reduce signal-to-noise. A critical issue can be easier to miss when it is surrounded by a dozen small comments.

The SmartBear/Cisco lightweight review study is about review size and attention, not specifically nits. Still, it supports the same operational concern: reviewer attention is finite. The study recommends reviewing fewer than 200-400 lines at a time and keeping review sessions around 60-90 minutes. If attention is finite, spending it on repeated polish comments has a real opportunity cost.

Source-Backed vs. Inference

Some claims are directly source-backed:

  • Label comment severity.
  • Mark polish as Nit: or optional.
  • Do not seek perfect code before approval.
  • Use automation for objective style rules.
  • Keep reviews small enough for humans to review well.

Some claims are my inference from the sources and from engineering practice:

  • More than two nit comments is usually a smell.
  • New nits after a fix pass should usually be skipped unless they reveal a real maintainability issue.
  • Repeated nits should become automation or team guidance.
  • If a reviewer wants many tiny preferences changed, the real problem may be unclear style ownership.

That distinction matters. I did not find a serious universal standard that says “exactly N nits are allowed.” The better answer is a policy that protects flow and attention.

How Many Nits Are Enough?

Use this as a team default until you have better local data:

0 nits:
  Best default. If it is not worth the author's time, skip it.

1-2 nits:
  Fine if they are clearly labeled optional and improve readability.

3+ nits:
  Stop scattering comments. Batch them in one summary, automate them,
  or open a follow-up style discussion.

Only nits remaining:
  Approve. Do not request changes.

New nits after fixes:
  Usually stop. The review is converging; do not reset the loop.

The goal is not to ban taste. Taste matters. Names matter. Readability matters.

The goal is to make the cost visible.

Every nit spends at least four things:

  • Reviewer’s attention.
  • Author’s attention.
  • CI time.
  • Merge queue time.

Sometimes that cost is worth it. Often it is not.

A Simple Severity Ladder

Teams get into trouble when every comment has the same shape. Use a ladder like this:

blocker:
  Must fix before merge.
  Examples: bug, security issue, broken test, missing required behavior.

issue:
  Probably should fix before merge, but discuss if tradeoffs exist.
  Examples: confusing design, edge case, incomplete error handling.

suggestion:
  Improvement with clear value, but not mandatory.
  Examples: simpler API, clearer helper extraction, better test name.

nit:
  Tiny polish. Optional by default.
  Examples: wording, small naming preference, minor readability tweak.

fyi:
  Knowledge transfer. No action expected in this PR.

In review comments, that becomes:

blocker: This endpoint returns invoices by id but never checks account ownership.
That can expose one customer's invoice to another customer.

suggestion (non-blocking): This test name could describe the behavior instead
of the implementation detail.

nit: `rows` might be clearer as `invoiceRows`.

fyi: This helper has a similar shape to the one in `billing/retry.ts`.
No action needed here.

The label is not ceremony. It is a merge protocol.

The Reviewer Flow

Before leaving a nit, run this small decision tree:

Is this correctness, security, data loss, broken behavior, or missing tests?
  yes -> blocker or issue
  no  -> continue

Is this already covered by formatter, linter, type checker, or CI?
  yes -> do not comment manually; fix the tool
  no  -> continue

Is this a documented team convention?
  yes -> suggestion or issue, depending on impact
  no  -> continue

Will this materially help the next reader?
  yes -> nit or suggestion, clearly non-blocking
  no  -> skip it

Have you already left 2 nits?
  yes -> batch the rest or stop
  no  -> leave it as optional

The hardest step is the last one. Reviewers see many possible improvements because they are engineers. The mature move is choosing which improvements deserve the author’s time right now.

The Author Flow

Authors also need a policy. If your team treats all comments as mandatory, you will over-fix and keep the loop alive.

Try this:

For each comment:
  blocker -> fix or discuss before merge
  issue -> fix, discuss, or document the tradeoff
  suggestion -> accept if it improves the PR without expanding scope
  nit -> accept if it is trivial and you agree
  fyi -> acknowledge only if useful

Useful author replies:

Fixed, thanks.
Good catch. I fixed the ownership check and added a regression test.
Thanks. I am going to keep this name because it matches the API field.
Agree this could be cleaner, but I would rather handle it in a follow-up
because this PR is only wiring the migration.

You do not need to win every comment. You need the PR to converge with the right risks handled.

Code Example: Turning Repeated Nits Into Policy

If a nit repeats, it is no longer a review comment. It is missing automation or missing team guidance.

Here is a small TypeScript example that models the policy. It is not a production GitHub bot. It is a way to make the rule concrete:

type Severity = "blocker" | "issue" | "suggestion" | "nit" | "fyi";

type ReviewComment = {
  body: string;
};

function classifyComment(comment: ReviewComment): Severity {
  const label = comment.body.trim().split(":")[0]?.toLowerCase();

  if (label === "blocker") return "blocker";
  if (label === "issue") return "issue";
  if (label === "suggestion") return "suggestion";
  if (label === "nit" || label === "nitpick") return "nit";
  if (label === "fyi" || label === "note") return "fyi";

  // Unlabeled comments are ambiguous, so treat them as issues until clarified.
  return "issue";
}

function reviewDecision(comments: ReviewComment[]) {
  const severities = comments.map(classifyComment);
  const nitCount = severities.filter((severity) => severity === "nit").length;

  return {
    shouldRequestChanges: severities.some(
      (severity) => severity === "blocker" || severity === "issue",
    ),
    shouldBatchNits: nitCount >= 3,
    nitCount,
  };
}

The important part is not the function. It is the rule:

unlabeled feedback creates ambiguity
repeated nits create noise
blockers decide mergeability
nits do not

In a real team, this could become:

  • A PR review guideline.
  • A GitHub saved reply.
  • A Danger rule that warns when PRs are too large.
  • A linter rule for repeated style comments.
  • A PR template asking reviewers to separate blockers from non-blockers.

What To Automate

Automate anything objective and repeated:

  • Formatting.
  • Import order.
  • Unused variables.
  • Simple naming conventions.
  • Generated file checks.
  • PR size warnings.
  • Required tests.
  • Secret scanning.
  • Dependency policy.

Do not spend human review comments on things a deterministic tool can catch.

Human review is better spent on:

  • Does this meet the requirement?
  • Is the design understandable?
  • Are the tests meaningful?
  • Are the edge cases handled?
  • Is there a security or privacy risk?
  • Is this PR doing too many things at once?
  • Will the next engineer understand the change?

That split is the heart of healthy review:

machines:
  objective, repeated, mechanical checks

humans:
  judgment, context, design, risk, mentorship

A Team Policy You Can Copy

Here is a lightweight policy that fits most engineering teams:

Pull Request Review Policy

1. Label review comments:
   blocker:, issue:, suggestion:, nit:, fyi:

2. Only blocker: and issue: comments can prevent merge.

3. If the only remaining comments are nit: or fyi:, approve the PR.

4. Keep nits rare:
   - 0 is ideal
   - 1-2 is acceptable
   - 3+ should be batched or automated

5. Do not leave manual comments for formatter or linter issues.

6. If a nit appears in three PRs, convert it into:
   - a style guide rule,
   - a formatter/linter rule,
   - a snippet/template,
   - or a decision to stop caring.

7. If a review discussion goes back and forth more than twice,
   talk synchronously and summarize the decision in the PR.

This is intentionally boring. Boring policies are easier to follow.

When Nits Are Actually Worth It

Nits are not evil. A good nit can teach taste, prevent a small confusion, or make a future change easier.

Nits are worth leaving when:

  • The fix is tiny.
  • The comment is clearly optional.
  • The reason is understandable.
  • The suggestion improves the next reader’s experience.
  • The reviewer is not using the nit to block approval.

Example:

nit (non-blocking): `isReady` might be clearer as `hasLoadedProfile` because
this flag only tracks profile loading, not the whole page readiness.

That is useful because it explains the future-reader problem. It is not just “I like another name.”

When Nits Become Harmful

Nits become harmful when they:

  • Arrive unlabeled.
  • Block approval.
  • Keep appearing after each fix pass.
  • Reflect personal taste instead of team standards.
  • Distract from correctness, tests, or security.
  • Create a tone where the author feels managed by preferences.
  • Hide a bigger process problem, like no formatter or unclear style guide.

The smell is not “someone left a nit.” The smell is:

the PR is correct enough to merge,
but the team cannot stop discussing tiny preferences

That is the infinite loop.

AI Reviewers Make This More Important

AI code review tools can generate many comments quickly. That makes severity labels even more important.

An AI reviewer that leaves ten optional style comments is not helping if it trains humans to ignore the review. An AI reviewer that separates “possible security issue” from “naming preference” is much more useful.

For AI-assisted review, I would use an even stricter rule:

AI may suggest nits,
but humans should not be forced to resolve them.

AI should prioritize:
  security
  correctness
  missing tests
  risky edge cases
  unclear behavior

AI is cheap enough to produce endless polish suggestions. Human attention is not.

The Best Possible PR Is Reviewable, Not Perfect

The “perfect PR” is usually a myth.

A reviewable PR is better:

  • It has one clear purpose.
  • It is small enough to understand.
  • It explains why the change exists.
  • It has tests where tests matter.
  • It passes automated checks.
  • It makes important tradeoffs visible.
  • It does not mix unrelated refactors with behavior changes.

That kind of PR gives reviewers room to focus on the real question:

Should this change enter the system?

Not:

Can we keep polishing forever?

Open Gaps In This Article

This article relies on public engineering guidance, practitioner reports, and review-size research. The biggest gap is that I did not find a strong controlled study that isolates nit comments specifically and measures their effect on merge time, defect detection, or team morale.

Useful follow-up research:

  • Measure how many review comments in your team are blocker, issue, suggestion, nit, and fyi.
  • Compare time-to-merge for PRs with only blockers vs. blockers plus many nits.
  • Track how often a nit repeats and whether it should become automation.
  • Test a one-month “approve with optional nits” policy and compare developer sentiment.
  • Evaluate AI review tools by severity precision, not just number of comments.

If your team is stuck in nit loops, start small:

  1. Add comment labels for two weeks.
  2. Make nit: explicitly non-blocking.
  3. Approve PRs when only nits remain.
  4. Convert repeated nits into automation.
  5. Review your own comments before submitting: delete the ones that do not change the outcome.

The point is not to lower quality. It is to spend quality attention where it matters.

Sources