Tone, Disagreement and Receiving Review
Critique the code and what it will cost, never the person. Then the two harder halves: disagreeing with someone senior, and taking twenty comments on work you were proud of.
The requirement, the obvious build, and why it breaks
Every lesson starts where the work starts: someone asked for something, and the first implementation that comes to mind survives until the requirement changes.
How do I say a design is wrong without the author hearing that they are wrong — and what do I do when the person I disagree with outranks me?
A junior engineer stops opening pull requests before Friday because the same reviewer always leaves fifteen comments. A senior engineer has quietly stopped reviewing that person's work at all, because "it always turns into an argument".
Be nice. Soften everything, add an emoji, prefix with "nit:", say "maybe consider". Politeness solves the tone problem.
Softening changes the packaging, not the content. "Maybe consider not doing it that way :)" is still a claim about the person's judgement with no reason attached, and the author can still only comply or resent it.
- Softening changes the packaging, not the content. "Maybe consider not doing it that way :)" is still a claim about the person's judgement with no reason attached, and the author can still only comply or resent it.
- It also makes severity illegible. If every comment is equally gentle, the author cannot tell the naming suggestion from the one about a charge that will silently succeed, and the important one gets the same four-minute fix as the rest.
- "Nit:" gets used as a shield for comments that are not nits, and after a while for comments that are genuinely arguable — at which point the author has learned to ignore a prefix that sometimes matters.
- Politeness does nothing about the actual hard case, which is not rudeness. It is a reviewer who is senior, confident, and asking for something the author believes is wrong.
- And the reverse failure is real: some teams optimise so hard for comfort that nobody says the design is wrong, which is a worse outcome than bluntness (Changeability Is the Goal).
What limits the solution, and what must never stop being true
This domain leads with these two. A design that ignores its constraints is not a design, and an invariant nobody named is one nothing is protecting.
- Written review has no tone. Brevity from a busy reviewer is indistinguishable from contempt, and the author supplies the missing tone from their own mood.
- The people involved will work together for years; every review is also a deposit or a withdrawal from that.
- There is a real power difference, and pretending there is not does not remove it.
- The change is due Thursday. Every disagreement is happening under time pressure, which is when people are worst at this.
- A review comment must be answerable with facts, a reason, or a question. If the only available responses are compliance and resentment, it is not a review comment.
- The author's judgement is the default. A reviewer's preference does not outrank it without an argument.
- Disagreement must terminate. A thread that cannot end is a design conversation happening in the wrong place.
Who owns what, and where the seams fall
Responsibilities decide boundaries; boundaries decide what an interface has to say.
- The reviewer owns naming a consequence. Not a preference, not a principle, not a book — what this will cost, in this codebase, and how they would know if they were wrong.
- The reviewer owns marking severity honestly, so the author knows which two of the fifteen comments actually matter.
- The author owns responding rather than complying. Silently changing code you believe is right leaves a structure in the codebase that nobody can defend (Architecture Decision Records).
- The senior person owns making disagreement cheap, because they are the only one who can. "Push back on me" is only true if it has visibly happened.
- The team owns escalation paths, so that an unresolvable thread has somewhere to go other than seniority (Design Review).
- The boundary is between the code and the author. "This swallows the timeout" is about the code. "You always swallow errors" is about the person, and there is no technical answer to it.
- The boundary between review and design discussion: once a thread has three rounds, it is a design conversation trapped in the wrong medium, and moving it out is the correct move rather than a defeat.
- The boundary between disagreement and escalation: escalate the decision, never the person. "We disagree about whether this needs a factory, can you break the tie" is fine; "he keeps blocking my PRs" is a different conversation with a different audience.
The same objection, twice
The difference between these is not politeness. The second set is arguably blunter. The difference is that every comment on the right names something in the code and something it will cost, so the author has somewhere to go other than agreement.
"Why didn't you use the existing validator? Did you look?" "This isn't how we do things here." "I've told you before about swallowing errors."
"There's a validator in billing/validate.ts that already handles the currency case. It isn't discoverable — I only know because I hit it last month. Reuse it, or is this case different?" "Handlers elsewhere delegate to the module rather than querying directly; mixing both means a schema change now has two kinds of caller to find. Not blocking, but it's why the convention exists." "This catch swallows the provider timeout, so a failed charge shows as a completed order. Can it propagate to the error boundary instead?"
Each comment on the right names a consequence — a rule duplicated, a schema change with two kinds of caller, a charge that fails while looking successful — and each is something the author can accept, refute or explain with a fact about this codebase. The comments on the left make claims about the author, which have no technical answer, so the only available responses are compliance and resentment. Note also that the middle comment marks itself as non-blocking: severity is information the author needs in order to spend their attention correctly.
Disagreeing with someone senior
The hard case is not rudeness. It is a reviewer with more experience, more context and more organisational weight asking for something you believe is wrong — where the cheapest move is to comply, and complying is how codebases acquire structure nobody can defend.
The move that works is to classify the disagreement before arguing it, because the five kinds have completely different resolutions and only two of them are winnable in a thread.
- The first and fourth are where a junior engineer has the most leverage and uses it least.
- A senior reviewer who cannot be moved by the first category is a different problem, and it is not a review problem.
- If a thread reaches three rounds without converging, the medium is wrong. Fifteen minutes on a call, then write the outcome back into the thread so the decision survives (Architecture Decision Records).
What kind of wrong is it, and what evidence do you actually have?
when They say the rule lives in one place; you know it is in four.
cost Cheap and decisive — post the four. Facts about the code end disagreements faster than anything else, and this is exactly the case where seniority is irrelevant, because they simply have not seen the other three.
when They expect a second payment provider; you do not.
cost Neither of you can settle it, and arguing it harder does not help. Name the trigger that would decide it, and pick the option that is cheaper to reverse (Reversible and Irreversible Decisions).
when You both know what the abstraction does; you disagree whether it is worth the indirection.
cost A legitimate cost disagreement and usually unresolvable in a comment thread. Merge the smaller version now and move the argument to a design discussion where it has room (The Cost of Change).
when The suggestion makes no sense to you at all.
cost Ask before arguing. "What breaks if we do not?" is not a concession and it has the best expected value of anything in this list — most of the time there is a reason, and occasionally asking for it is what reveals there is not.
when "This should be a factory." Why? "Separation of concerns."
cost Ask which specific change it makes cheaper. If no answer comes, say you prefer the simpler version and would defer to a team convention if there is one. Do not escalate this; it is not worth the capital, and the honest framing of "we do it this way here" is an answer you can accept (Changeability Is the Goal).
Receiving it
The half of this that is never taught. A diff you wrote is close to being a piece of you, and twenty comments on it arrive as twenty small verdicts even when every one of them is about the code.
The failures below are ordinary and mostly invisible from the reviewer's side, which is why the response column matters more than the diagnosis.
| Trigger | Symptom | Cause | Response |
|---|---|---|---|
| Twenty comments on work you were proud of | You answer every one, at length, defending each choice | Volume reads as a verdict, and the natural response to a verdict is a defence | Batch it. Accept the cheap ones without argument, pick the two that actually matter, and spend the disagreement budget there (A Review Checklist Worth Reading). |
| A comment you believe is wrong | You silently change the code to close the thread | Compliance is faster than disagreement, especially upward, and the deadline is Thursday | The codebase now contains a structure nobody can defend, including the reviewer. Ask what change it makes cheaper and record the answer in the thread, so the next person inherits a reason instead of a mystery (Architecture Decision Records). |
| Three rounds of changes requested | Still not approved, deadline moving, both people frustrated | The disagreement is about design, and design cannot be resolved a comment at a time | Stop the thread. Talk for fifteen minutes, then write the outcome back into the pull request — and consider whether this should have been a design review before the code existed (Review as Design Feedback — and Why It Arrives Too Late). |
| A comment reads as hostile | You reply in kind, or go quiet for a day | Written review strips tone, and brevity from a busy reviewer is indistinguishable from contempt | Take the least dramatic reading available and answer the technical content. If it is genuinely a pattern rather than a reading, that is a conversation with a manager and not a reply in a thread. |
| No comments at all, approved in ninety seconds | Relief | The pull request was too large to review, or you are the only person who understands the subsystem | Not a compliment and not a review. Slice it next time (Review Size), or ask someone to pair on the subsystem so that a second person can form a view at all (Knowledge Sharing). |
| The same reviewer always finds something | You start opening pull requests only when they are away | Review has become a status interaction rather than a technical one, on one side or both | Name it early and specifically — "I get more comments than others and I want to understand what I am doing differently" — because the alternative is that it is resolved by avoidance, which costs the team a reviewer and costs you the feedback. |
How to build it
Most important first.
- Say what it costs. Every substantive comment should contain a consequence: a rule in two places, a charge that looks successful when it is not, a boundary the next change will have to cross. That is the part the author can accept or refute.
- Ask before asserting. "Was this deliberate?" costs nothing and is correct surprisingly often, because the author usually knows something about the surrounding system that the reviewer does not.
- Mark severity explicitly and use the marks honestly — blocking, worth discussing, take it or leave it. Three levels is enough, and the top level should be rare enough that it means something.
- Own the preferences as preferences. "I would have done X, but this is fine" is honest, useful and does not oblige anyone. Dressing it as a principle is what makes it corrosive (Pattern Overuse).
- Praise specifically and rarely. "Nice" is noise; "this made the failure case obvious, I could tell what happens on a timeout without asking" is information about what the team values.
- When you are the author: separate the comments you disagree with from the ones you simply do not like, respond to the first group with reasons and accept the second group without argument. The disagreement budget is small — spend it on the two comments that matter (A Review Checklist Worth Reading).
What the next change costs
The field this whole domain exists for. A structure is only better if it makes the change after this one cheaper — and it is worth saying which changes it does not help.
- Under review-as-argument: the next change costs a day of thread, one person quietly complying, and a structure in the codebase that its own author would not defend. That structure is what makes the change after it expensive.
- Under consequence-naming review: the next change costs a comment, a reason, and either a fix or a recorded rationale. Both outcomes leave the next reader better off than before.
- The compounding cost is in who is willing to review whom. A team where two pairs of people avoid reviewing each other has lost the knowledge-sharing purpose of review entirely, and no cycle-time metric will show it (Bus Factor).
- What does not get cheaper: naming consequences takes longer to write than "use a factory here", and the reviewer pays that cost on every comment, forever.
- Naming a consequence for every comment is slower and sometimes pedantic. Experienced reviewers whose instincts are usually right will find it a tax, and some of that tax buys nothing.
- Explicit severity marks can be gamed in both directions — everything blocking, or nothing blocking — and a team that games them has a trust problem the marks will not fix.
- Making disagreement cheap makes some disagreements happen that would have been better dropped. That is a genuine cost, and the alternative is worse.
What can go wrong
- The reviewer is right and unpleasant, so the author complies without understanding, and the same class of comment recurs forever. Both people conclude the other is the problem.
- The author is right and junior, so the wrong structure ships. Nobody records why, and two years later it is a convention (What Technical Debt Actually Is).
- Conflict avoidance: the reviewer sees the design problem, calculates the cost of raising it, and approves. This is the most common outcome and it is completely invisible.
- The mitigation fails too. A team that adopts "always assume good intent" as a rule uses it to suppress the report of a genuine pattern, and the person experiencing it now has no available move.
- Escalation becomes personal. Once a technical disagreement has been framed as a conflict between two people, no technical resolution ends it.
- This depends on the team having a shared standard for what makes a design argument, otherwise every disagreement is preference against preference and seniority is the only tiebreak available (Changeability Is the Goal).
- It depends on the mechanical comments being gone. A reviewer who leaves fifteen comments of which thirteen are style has created a tone problem out of a missing configuration file (What to Automate Out of Review).
- It depends on review not being a gate someone can hold indefinitely. Where one person can block indefinitely, tone advice is beside the point — the structure is the problem (Code Ownership).
- "So be indirect." No — be specific. Directness is fine and often kind; the thing to remove is the claim about the person, not the clarity.
- "So the author should just accept feedback gracefully." Half the responsibility is the reviewer's, and telling authors to be resilient is how a team avoids fixing its review culture.
- "Disagreeing with a senior engineer is career risk." Sometimes it is, and that is a fact about the organisation rather than about review. Where it is true, the useful moves are to ask questions rather than assert, and to make the disagreement about the code in public and the pattern in private.
- "This is soft-skills content, not engineering." The output of review is what gets merged. A review culture where nobody contradicts the loudest person produces a codebase shaped by volume, which is an engineering outcome (Bus Factor).
Testing it, and how it ages
- The falsifiability test on your own comment before posting it: what would the author have to show you for you to withdraw this? If the answer is nothing, it is a preference, and it should say so.
- The reversal test: would you write this comment the same way to the most senior person on the team? If not, that difference is the thing to look at.
- For a team: check whether junior authors ever push back successfully in threads, and whether the record shows a senior reviewer changing their mind. If neither appears in six months of history, the stated openness is not real.
- As a team matures, more of the disagreement moves out of pull requests and into design discussion, which is the correct direction — review is a bad medium for design arguments and always was (Review as Design Feedback — and Why It Arrives Too Late).
- As people join, the norms have to be made explicit. What a five-person team does by instinct is invisible to the sixth person, and their first three reviews teach them what the team is actually like (Knowledge Sharing).
- Distributed and asynchronous teams need this harder, because the corrective mechanisms — reading a face, a two-minute conversation — are not available and the thread is all there is.
- What forces a rethink: someone leaves and says in their exit interview that review was the reason.
Where this applies
This domain's advice is contested more than most. These labels say what each claim is specific to — and where CONTESTED appears, the note gives the strongest form of the opposing view rather than a caricature.
- GENERALNaming a consequence rather than a person is a property of the argument, so it transfers across teams and stacks; what changes is how much directness is read as hostility, which varies enormously by team culture and by the language people are writing in.
- SCALE-SPECIFICOn a team of four who talk daily, most of this is handled by walking over; the written comment carries little weight because the conversation is available. At fifty, distributed across time zones, the thread is the entire relationship and every ambiguity in it is resolved by the reader's mood.
- CONTESTEDThe strongest opposing position is that tone norms are a form of politics that protect weak work: teams with famously blunt review — kernel and compiler communities are the usual examples — have produced some of the most reliable software in existence, and the argument is that softening review lowers the bar because nobody wants to be the one who says the design is bad. It is not a frivolous position and it has real evidence behind it. The counter is selection: those projects retain the people who tolerate that mode and lose the rest, which is affordable for a volunteer project with unlimited applicants and rarely affordable for a company.
Where the depth lives
This domain teaches the codebase-level structure and hands the rest off.
- — System Design — the disagreements that genuinely cannot be settled in a pull request are usually system-level trade-offs wearing a code-level costume, and they need a forum with room for them.