CodeCrafting All articles
Engineering Culture

Pull Requests Without the Mumbling: How to Review Code Honestly and Kindly

CodeCrafting
Pull Requests Without the Mumbling: How to Review Code Honestly and Kindly

Photo: Baltimore Heritage from Baltimore, MD, USA, CC0, via Wikimedia Commons

There's a particular kind of pull request comment that only really happens in British teams. It goes something like: "I might be completely wrong here, and feel free to ignore, but I just wondered if perhaps there might be an argument for possibly naming this slightly differently? Sorry!"

It's polite. It's well-meant. And it's completely unusable, because the author has no idea whether they're being asked to change something or being wished a pleasant afternoon. So they change it, slightly, and everyone moves on having learned nothing.

Why reviews go quiet

British workplace culture rewards the stiff upper lip. Disagreement is softened, criticism is wrapped in three layers of hedging, and directness can read as rudeness. That's fine at the tea point. In a code review it's a bug.

The failure modes are consistent:

All four cost the team time. The fix isn't to become blunt — it's to become specific.

Draw the line first: block, discuss, or nit

Most review friction comes from ambiguity about what kind of comment you're leaving. Fix that with a shared taxonomy, agreed once, written down, and used consistently.

Then prefix every comment with its type: blocking:, discuss:, nit:, praise:. It takes four keystrokes and removes almost all of the guesswork. The author can triage in seconds; the reviewer commits to a position rather than hovering near one.

Two rules keep the taxonomy honest. First, nits never block — if you genuinely can't merge until it's fixed, it wasn't a nit. Second, if you're leaving more than a handful of nits, automate them with a linter and stop spending your colleagues' attention on them.

Templates that do the heavy lifting

Templates sound bureaucratic; in practice they're a scaffold that makes directness easier, because the structure is doing the confronting rather than you.

A blocking correctness issue:

blocking: This retries on timeout without an idempotency key, so a duplicated webhook will charge the customer twice. Could we key on event_id?

A genuine question:

discuss: Is there a reason we're doing the lookup here rather than in the caller? Asking genuinely — I might be missing context.

A non-blocking suggestion:

nit: This function is doing three things. Not blocking at all, but splitting out the invoice lookup would make the tests easier. Your call.

A small fix:

nit: recieve → receive. Happy either way.

Praise that means something:

praise: The jitter on this backoff is exactly right — saved me a search.

And the one people avoid:

blocking: I don't understand what this does. That's as much on me as on the code — could we rename it or add a comment?

Note the last one. "I don't understand" is a blocking comment, and it should never be softened into a question mark and a shrug. If a competent colleague can't follow the code, the code is the problem. Saying so plainly is a kindness, not an insult.

Kindness is specificity

Here's the counterintuitive bit: vague praise is as unhelpful as vague criticism. LGTM teaches nobody anything. So does this is wrong.

Specificity is what makes feedback land without bruising. "This is bad" is an attack on a person. "This function mutates the argument it's passed, which surprised me" is an observation about code. One starts an argument; the other starts a fix.

And resist the urge to soften a blocker into invisibility. If something will page someone at 2am, saying "just a thought, no pressure" is not kindness — it's offloading the awkwardness onto the author, who now has to guess whether it matters.

Norms that keep things quick

A few small team agreements do more than any amount of exhortation:

There's also the tea-round principle: everyone reviews, everyone gets reviewed, nobody is the designated approver-hero. Review load is shared because review is part of the work, not a favour.

What changes

Teams that adopt this tend to notice the same things. Pull requests get reviewed faster, because reviewers aren't spending ten minutes composing a sentence that conveys almost nothing. Fewer concerns surface after release, because they surfaced in the diff instead. And the tone improves — not because people are being nicer, but because they're being clearer, and clarity is what most people actually experience as respect.

You can be honest and you can be kind. In a good review culture, they're the same sentence.

All Articles

Related Articles

The Repair Café Mindset: Keeping Legacy Systems Alive Without the Grand Rewrite

The Repair Café Mindset: Keeping Legacy Systems Alive Without the Grand Rewrite

Designing for the 2am Debugger: What Your Change Looks Like When You're Asleep

Designing for the 2am Debugger: What Your Change Looks Like When You're Asleep

Same Bytes, Every Time: Reproducible Builds as Risk Management, Not Ritual

Same Bytes, Every Time: Reproducible Builds as Risk Management, Not Ritual