Code Review Best Practices

The single strongest predictor of review quality is diff size. A reviewer's attention degrades sharply past a few hundred changed lines, and large PRs invite the rubber-stamp approval - the worst possible outcome, because it carries the appearance of review without the substance.

• One intent per PR. A refactor and a behavior change should be separate PRs, even if they touch the same files. Mixing them hides the risky change inside the noise of the safe one.
• Split mechanical from meaningful. Rename-only or move-only commits should land on their own so reviewers can skim them and focus on logic elsewhere.
• Stack dependent PRs rather than piling everything into one branch. Small, ordered PRs review faster than one giant one.

If a PR is unavoidably large, say so in the description and tell the reviewer where to look first. See cycle time explained for why small batches move faster end-to-end.

The diff shows what changed; only the author knows why. A good description turns a review from archaeology into a conversation.

• State the problem being solved and link the ticket or incident.
• Explain the approach and any alternatives you rejected - this pre-empts half the review comments.
• Flag the risky parts yourself. "The retry logic in PaymentService is the part I'm least sure about" directs attention where it matters.
• Note how it was tested, and whether any of the code was AI-generated so the reviewer calibrates their scrutiny.

Consistent, structured commit messages make the history readable long after the PR closes - see the conventional commits guide.

Reviewers should never be the first line of defense against formatting, style, or obvious type errors. Every minute spent on something a tool could catch is a minute stolen from reasoning about correctness.

• CI gates for linting, formatting, type checks, and the test suite must be green before human review starts.
• Coverage and static analysis surface untested branches so the reviewer can ask about them directly.
• Automated PR summaries help orient large diffs, but treat them as a map, not a verdict.

When the machine has done its job, the human can spend their whole budget on judgment. Try the code review checklist to standardize what "ready for review" means.

Separate what must change from what you would have done differently. Blocking a PR over stylistic taste erodes trust and slows the team; reserve blocking comments for real defects.

• Correctness: Does it handle nulls, empty collections, concurrency, and failure paths? Are error cases swallowed or surfaced?
• Boundaries: Are inputs validated? Are permissions and auth checks intact? Any injection or unsafe deserialization?
• Blast radius: What happens to existing callers, data, and migrations? Is the change backward-compatible?
• Clarity: Will the next engineer understand this in six months without the author present?

Label non-blocking comments explicitly - a "nit:" or "optional:" prefix tells the author they can merge without addressing it. This one habit measurably cuts back-and-forth.

AI-generated code is fluent and confident, which is exactly what makes it dangerous to review. It rarely has syntax errors or style problems; it fails on assumptions, hallucinated APIs, and edge cases the prompt never mentioned. Skimming it is the trap.

• Verify the APIs exist and behave as used - generated code will invent plausible method signatures and config keys.
• Check the tests test something. AI often writes tests that assert on the implementation rather than the behavior, or that pass trivially.
• Hunt for silent scope creep - generated diffs sometimes "helpfully" change unrelated code or drop existing edge-case handling.
• Confirm the author understands it. The person opening the PR owns the code regardless of who wrote it; if they can't explain a section, it isn't ready.

See AI vs human code quality and measuring AI coding tools for how to track this over time. The AI vs human PR analyzer helps spot where generated changes concentrate risk.

Latency matters more than most teams admit. A PR that sits for a day blocks the author, invites merge conflicts, and drains momentum. Aim for a first response within a few hours, even if that response is "I'll do a full pass this afternoon."

• Be concrete. "This will NPE when items is empty" beats "handle edge cases." Point at the line and the scenario.
• Ask, don't decree. "What happens if this retries after a timeout?" invites a fix and surfaces intent better than an order.
• Switch to a call once a thread hits three round-trips. Some disagreements are faster resolved by voice.
• Approve with confidence. If the remaining comments are nits, approve and trust the author to clean them up - don't hold the PR hostage.

Track time-to-first-review and review depth to see whether your process is fast without becoming shallow.

A review process you can't see is one you can't improve. The point of measurement is not to rank reviewers - it's to find where PRs stall and why some slip through with too little scrutiny.

• Time-to-first-review and time-to-merge tell you where latency lives.
• PR size distribution reveals whether small-batch habits are actually sticking.
• Review coverage - the share of PRs with substantive discussion versus instant approvals - flags rubber-stamping before it bites.
• Rework and revert rates connect review quality to outcomes in production.

These roll up into broader delivery health; see what are DORA metrics and PR quality analytics for the full picture.