Sarthak Garg

Code review culture

A culture document written in reviewer comments.

·8 min read·

Most teams have a wiki page about how they do code review. It says things like "be kind," "explain your reasoning," "approve when the change is good enough." Almost nobody reads it after their first week. Every day, people read something else: the comment stream on open pull requests.

That stream is your real culture document. The wiki describes the team you wish you had; the last fifty review threads show the team you have: how disagreement gets handled here, whether a question is welcome or treated as an attack, whether good work ever gets named, whether a junior can hold the line against a senior. New hires learn all of this in their first two weeks, and they learn it from the threads, not the wiki page.

You can watch it happen. A new engineer joins, reads through a few merged PRs to learn the codebase, and picks up the tone along with the code. Three weeks later their own review comment on a teammate's change is a single word with no explanation: "why?". They are not doing it to be rude; they are copying the senior reviewers, whose comments look exactly the same.

As the leader, you are the editor of that document whether you choose to be or not. Every norm in the stream is one you set, modeled, or tolerated. You cannot disown it by pointing at the wiki.

Audit the stream first

Before you rewrite the review guidelines, read the existing threads. Open the last fifty merged PRs and read the comments as if you were a new hire trying to figure out the rules.

You are looking for patterns rather than individual slip-ups:

  • Does every approval come with a "looks good" and nothing else, which means review is just for show and nobody is reading?
  • Do certain reviewers redesign every change they touch?
  • Are real bugs and style preferences arriving in the same flat tone, so the author cannot tell which is which?
  • Is there a reviewer everyone routes around because their threads are exhausting?

The wiki cannot tell you any of this. The threads can. Once you have read fifty of them, you will know your review culture better than any document, and you will know exactly where to start.

Label how much each comment matters

When people complain about review, they rarely mean the feedback is harsh. They mean it all sounds the same. A correctness bug and a preference about variable naming show up as two comments in the same thread, in the same flat voice, and the author cannot tell which one they have to fix and which one is just a preference.

Most teams fix this by labeling each comment by weight: blocking, a suggestion, or a nit. Blocking stops the merge, for a correctness bug, a security hole, or a real design problem. Everything else is non-blocking by default, and the author decides which nits to take before shipping.

Picture the alternative. A PR comes back with fourteen comments, two of them genuine bugs and the other twelve about spacing, naming, and "I would have done this differently." The author cannot tell which of the fourteen matter, so they do the safe thing and address all of them, rewriting working code to satisfy preferences and maybe adding a new bug along the way. The two real issues carried the same weight as an argument about a variable name.

These labels are not about politeness. The wording of a hard comment is its own skill. They do something narrower: they tell the author what they have to fix and what they can ignore. Reviewers have to honor that, so a nit can never quietly block a merge. The first time a nit holds up a release, nobody trusts the labels again.

Hold the bar at code health

Without a clear standard for when to approve, every reviewer falls back on one question: is this how I would have written it? The strongest teams swap that for a less obvious one: approve a change once it makes the codebase better than it was, even when it is not perfect. Nothing ever is, and holding out for perfect just means nothing ships. When a reviewer's taste runs into a plain fact about the code, the fact wins. Aim for code that gets a little better with every merge.

You will have to defend that standard, because one kind of reviewer pushes hard against it: the one who would rebuild every change around a different design.

A junior opens a correct, tested change. A senior reviewer would have built it around a different abstraction, so they ask for the rewrite. The PR sits for two days while the author redoes something that already worked, and everyone watching learns that being right is not enough here; you also have to guess the reviewer's taste.

When a reviewer prefers a different design, they can raise it as a follow-up; it does not block a change that already works. If the change improves the codebase and is not buggy, it ships, and the team can take up the better design on its own. Reviewers can abuse this from the other side, too. One waves a change through, says it "improves code health," and never reads it closely. So reviewers have to read for it, or the standard means nothing.

Hand the machine its arguments

No human should ever write a whole class of review comment: formatting, import order, line length, whitespace, the lint rules, the obvious security scans. A machine should settle all of them. If two engineers are debating spacing in a thread, your toolchain has a hole in it.

Push style, linting, and security scanning into automation that runs before a human looks at the change. This does not lower the bar. It points human attention at what only people can judge: the logic, the architecture, the intent, whether the change should exist at all. Take those nits off the table and the reviewer gets their attention back for the questions that need a person.

You can also trust the toolchain too much. A green linter does not mean the logic is sound. A machine owns the arguments that have one correct answer, but it does not own judgment. Pretend otherwise and a passing build will ship you a broken feature.

Watch how long changes wait

Watch how long a change waits before anyone looks at it. When a PR sits for a day before its first comment, the author hears that their work does not matter to anyone.

Set a norm on response time, and be precise about what it means: you promise to look within a few hours, not to approve within a few hours. Looking quickly keeps you honest about attention; approving quickly just pressures everyone to rubber-stamp. Pair it with an expectation about change size, because a review only works if it is small enough to hold in your head. Then watch how long that first response takes, and when it starts climbing, something is wrong before any other signal shows it.

As more code gets generated, more changes pile into the review queue than ever before. The mechanics of that surge are their own subject. You own the cultural half of it: when the queue is slow, people learn that review does not matter here.

Write the first chapter yourself

You undo all of it the moment you exempt your own code from review. You write the first chapter of the culture document, because everyone watches how the leader behaves in review far more closely than they listen to anything the leader says about it.

Put your own changes through the same review, in the open, with no special lane. Ask instead of command in your comments, because "could we pull this into its own function?" and "pull this into its own function" teach two different cultures. And when someone junior catches a real bug in your code, the team will remember how you respond to it in public.

A junior leaves a comment on your change pointing out a case you missed. You reply in the thread, thank them by name, and ship their fix. Everyone watching just learned that catching the leader is safe, and that the leader answers a catch with thanks instead of a defense.

You corrupt the whole document fastest by doing the reverse: never letting your own code get reviewed, or arguing down every comment on it. Then the first chapter says the rules do not apply to you, and everyone has read it.

What the stream has to stay honest about

When reviewers only ever flag what is wrong, people learn to dread review. It becomes the place their work goes to get marked up and never praised. So name good work too, and say why: this is a clean abstraction, this test is exactly the one I would have wanted, this is much easier to follow than the last version. Skip the empty praise that inflates until it means nothing. Point at something specific that cleared the bar, and the team sees where it sits.

The critics of review get something right. It can turn into gatekeeping, a status contest where authors write to please the reviewer instead of improving the code, and where the slow back-and-forth between author and reviewer eats up everyone's day. That risk is real, and every section above is aimed at it. But the answer is not to scrap code review. Teams do not get into trouble by writing their culture down in review comments. They get into trouble by leaving it unedited. You are the editor whether you want the role or not, and the stream is already being written. Read it back, and fix what it teaches.