Code review is one of the most leveraged activities in a software team. Done well, it raises code quality, spreads knowledge, and helps people grow. Done poorly, it bottlenecks every release, demoralizes contributors, and produces a culture where shipping feels adversarial.

The difference isn't in tooling or process. It's in habits. This is a practical guide to the ones that work.

Why are we even doing this?

Before diving into how to review, it helps to be honest about the why. A useful framing: code review serves four goals, in roughly this priority order.

  1. Catch bugs before they ship. This is the surface-level reason and the least important.
  2. Spread knowledge across the team. Every PR is an opportunity for two or three engineers to learn something about a part of the codebase.
  3. Maintain consistency. Without review, a codebase becomes a museum of personal preferences.
  4. Raise the floor. Good review nudges every engineer toward better practices over time.

If your code reviews focus only on goal #1, you're getting a small fraction of the value.

What good review looks like

A solid review touches three layers:

  • Correctness: does this code do what it claims to do? Are there bugs, edge cases, or security issues?
  • Design: is the change in the right place? Are the boundaries right? Will this hold up next quarter?
  • Style: is the code readable, consistent, and maintainable?

Most teams over-index on style and under-index on design. Style is the easiest to spot, the most fun to comment on, and the least valuable. Design is the hardest to spot, the most awkward to push back on, and the most valuable.

A useful self-check before submitting comments: am I spending most of my energy on the highest-leverage layer?

How to give a review that helps

Read the description first

If the PR has a real description, read it before opening the diff. Understanding the why changes how you read the what.

If the PR has no description, that's the first comment to leave. "Could you add a brief description of the goal and how to verify?" — every time. The team learns the norm.

Read the whole diff before commenting

Reviewers who comment file-by-file as they go often miss the bigger picture. A function that looks weird in users.ts makes sense once you see how it's used in auth.ts. Read the whole thing once, then go back and comment.

Distinguish blocking from non-blocking comments

Make your review comments easy to triage. A simple convention works:

  • nit: — a minor style preference. The author can ignore it.
  • question: — a real question. Not a request to change anything.
  • suggestion: — a stronger nudge. The author should consider it.
  • blocking: — must be addressed before merge.

This single convention removes hours of "is this person asking me to change it or just commenting?" confusion across a quarter.

Suggest, don't decree

Compare:

"This should use useCallback for performance."

vs.

"Curious if useCallback would help here — would <ChildComponent /> re-render unnecessarily otherwise?"

The first feels like an order. The second invites a conversation. They take roughly the same time to write and produce dramatically different reactions.

Praise the good parts

If a function is unusually clear, a test cleverly designed, a refactor improves the surrounding code — say so. Praise costs nothing, takes ten seconds, and changes how reviews feel for the author.

Be specific

"This is messy" is not feedback. "This function is doing three things — fetching, formatting, and dispatching. Consider splitting formatPost out so we can test it independently" is feedback.

If you find yourself writing a vague critique, stop and ask: what would the better version look like? Write that.

How to receive a review well

The author has just as much responsibility for the review going well as the reviewer.

Make your PR easy to review

The single biggest thing you can do is keep PRs small. Under 400 lines of diff is a good target. If your PR is bigger, ask yourself if it can be split into pieces — almost always, the answer is yes.

Beyond size:

  • Self-review your own diff first. Half the comments you'd get, you'll catch on your own.
  • Write a description that includes context, the approach, and how to verify.
  • Leave inline comments on parts of the code that need explanation. Anticipate questions.
  • Mark drafts as drafts.

Don't get defensive

The reviewer is not attacking you. They're trying to make the code better. If a comment lands wrong, take a beat before responding.

When you disagree:

  • Try the suggestion privately. Sometimes the reviewer is right and you'll only see it once you write the alternative.
  • If you still disagree, explain why in the PR. Reasonable disagreement, written down, is how teams build judgment.
  • Escalate if necessary. Some technical disagreements aren't going to resolve in PR comments. That's fine — schedule fifteen minutes and talk it through.

Mark conversations resolved meaningfully

When you address a comment, leave a short note (Done in 4f5a2) and resolve it. Don't silently make changes. Don't argue and leave it unresolved.

Handling friction

Some patterns that defuse the most common review tensions:

  • The "can we discuss synchronously" call. Some discussions don't fit in PR comments. Hopping on a 10-minute call to talk through a design decision is almost always faster than thirty comments back and forth.
  • The "let's pair on this" offer. When a PR is heading into territory where the reviewer would basically rewrite it, pair-program instead.
  • The "this is a bigger discussion" pause. If a comment uncovers a real architectural disagreement, the PR isn't the right venue. Land the smaller change as is, file a separate doc to discuss the larger question.

The teams we've seen do code review well treat these as normal moves, not escalations.

Anti-patterns to avoid

A short list of behaviors that quietly poison review culture:

  • Drive-by comments on style. Twenty trivial nits with no acknowledgment of the actual change.
  • The 800-line PR opened on Friday afternoon with "small change" in the title.
  • Reviewers who never approve. "LGTM with these comments addressed" is a valid approval. Refusing to approve until perfection turns review into bottleneck.
  • Authors who address every comment with "I'll do that in a follow-up PR" that never comes.
  • Reviewers who rewrite the code in suggestion blocks. Sometimes appropriate. Often condescending.

The pattern across all of these is asymmetric effort: someone is doing more work than necessary because someone else isn't doing the work they should.

Norms worth setting explicitly

A few team-level decisions reduce friction substantially:

  • A response-time expectation. "We aim to give first review within 4 working hours" turns "waiting on review" into a measurable thing.
  • A "two approvers" or "one approver" policy. Pick one and stick with it.
  • A list of things that don't require review (typo fixes, doc-only changes, dependency bumps from a trusted bot). Free up cycles for the changes that need real attention.

Culture is just the sum of unwritten habits. Write a few of them down and the unwritten ones tend to follow.

A final note

Code review isn't a gate to keep bad code out. It's a conversation that makes the team better. The reviewers and the authors are on the same side. When that's clear in how the review is conducted, everything else gets easier — quality goes up, frustration goes down, and the team becomes the kind of place where strong engineers want to stay.

Get that part right, and the rest of code review takes care of itself.