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:
- The hedge. Feedback so qualified that nobody can tell if it's a blocker.
- The apology. Reviewers pre-emptively sorry for commenting at all, which quietly trains people to comment less.
- The silence. A pull request gets an LGTM in four minutes and a raised eyebrow in the retro a fortnight later.
- The ambush. None of the concerns surfaced in the review, all of them surface after the release.
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.
- Blocking — correctness, security, data loss, broken contracts, anything that would page someone. This must be resolved before merge.
- Discuss — design trade-offs and structural choices. The author should respond, but they may well convince you.
- Nit — style, typos, personal preference. Explicitly never blocks merge.
- Praise — something done well. Not filler. Genuinely useful signal.
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 onevent_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:
- First response within a working day. Ideally a couple of hours. A pull request sitting untouched for two days is a context switch waiting to happen.
- Under about 400 lines where possible. Nobody reviews a 2,000-line diff properly, whatever they claim.
- Review your own PR first. Walk the diff, leave a comment on anything non-obvious. It halves the questions and shows you've thought about the reader.
- Two reviewers maximum. Beyond that it's a committee.
- Three replies, then a call. If a thread has gone back and forth three times, the medium has failed. Fifteen minutes on a video call beats an afternoon of increasingly careful prose.
- Request changes sparingly and mean it. If you're blocking, say what would unblock it.
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.