Chapter 6: Giving Feedback Well: Nits vs. Blocking Issues, Tone, Positive Intent

Why this matters

The technical content of a review comment and its actual effectiveness are two different things — the same underlying concern, phrased two different ways, can either be received as helpful information or as an attack, and this single factor determines more about whether a team's review culture builds trust or erodes it than any process or tooling decision covered elsewhere in this book.

Distinguishing nits from blocking issues, explicitly

[BEGINNER] A nit (nitpick) is a minor, non-blocking preference — worth mentioning, but not worth holding up the PR over. A blocking issue genuinely must be resolved before merging. Conflating the two — treating every comment as equally mandatory, or leaving a reviewer to guess which category a comment falls into — creates unnecessary friction and unclear expectations. Most review tools support explicit labeling, and using it consistently removes ambiguity entirely:

nit: Consider naming this `userCount` instead of `count` for clarity —
     not blocking, up to you.

BLOCKING: This doesn't handle the case where `items` is empty, which will
throw an unhandled exception in production based on the calling code in
OrderService. Needs a fix before merge.

[SENIOR INSIGHT] Labeling every single nit explicitly (rather than letting an author guess) has a real, underappreciated cultural effect: it signals that the reviewer has already distinguished "must fix" from "your call," respecting the author's time and autonomy on genuinely optional matters — an author who has to guess which of ten comments are mandatory either over-corrects everything (wasting time on non-issues) or under-corrects (missing something that was actually intended as blocking), and both outcomes stem from the same root cause: ambiguity the reviewer could have resolved with one word.

Weak vs. improved: the same technical concern, different tone

[BEGINNER] The single most valuable skill this chapter teaches: rephrasing the same technical observation to be received well, without softening or hiding the substance of the concern.

Weak — accusatory, unclear about severity, unhelpful:

This is wrong. You clearly didn't think about concurrent access here.

Improved — same concern, specific, non-accusatory, actionable:

BLOCKING: This check-then-act sequence (checking `stock_quantity` then
updating it) isn't protected against concurrent requests — two simultaneous
purchases could both pass the check before either updates the count,
allowing overselling. Wrapping this in a transaction with `SELECT ... FOR
UPDATE` would close the race (see Database guide, Chapter 3, for the exact
pattern). Happy to pair on this if useful.

Weak — vague, provides no actionable path forward:

This code is kind of messy.

Improved — specific about what "messy" means and what would improve it:

nit: This function is handling three different concerns (validation,
calculation, and persistence) in one place — splitting it into three
smaller functions would make each one independently testable and easier
to follow. Not blocking, but worth considering as a follow-up if you agree.

Weak — assumes bad intent, phrased as a personal criticism:

Why would you do it this way? This makes no sense.

Improved — assumes positive intent, asks rather than accuses:

I'm not sure I understand the reasoning for this approach — could you walk
me through why this pattern was chosen over [alternative]? I might be
missing some context, but my initial read suggests [specific concern].

Assuming positive intent as a default operating stance

[SENIOR INSIGHT] The consistent thread across every "improved" example above: assume the author had a reason for their choice, even one that turns out to be wrong, and phrase feedback as if that reason exists and is worth understanding, rather than assuming carelessness or incompetence. This isn't just politeness for its own sake — it's frequently more accurate: an author often does have a reason (a constraint the reviewer doesn't know about, a misunderstanding that's genuinely easy to have, a deliberate trade-off) and asking about it surfaces that reason, while accusing first forecloses the conversation that would have revealed it. Even when the author genuinely made a straightforward mistake, phrasing feedback as a question ("did you consider X?") rather than an accusation ("you forgot X") produces the identical corrective outcome with none of the relational cost.

Comment specificity: point at the exact problem, don't make the author search

[IN PRACTICE] A comment that identifies a category of problem without pointing at its specific instance forces the author to do the reviewer's diagnostic work over again:

Weak: "There might be some edge cases you're missing."

Improved: "What happens if `items` is an empty array here? Looking at the
loop below, I think this would cause a division by zero on line 42."

The improved version required more effort from the reviewer to write, but saves the author from having to independently rediscover exactly what the reviewer already found — this trade is almost always worth making, since one reviewer writing a specific comment once is cheaper than the author separately re-deriving the same finding.

Common mistakes

1. Leaving every comment unlabeled, forcing the author to guess which are blocking and which are optional nits — creating unnecessary anxiety or, conversely, missed fixes.
2. Phrasing a technical concern as a personal criticism ("you clearly didn't think about X") rather than a specific, depersonalized observation about the code — the same substance delivered destructively rather than constructively.
3. Writing vague comments ("this seems off," "not sure about this") that don't identify the specific problem or suggest a specific direction, forcing the author to guess what's actually being flagged.
4. Assuming carelessness or incompetence by default rather than assuming the author had a reason worth understanding — this both damages trust and, often, misses learning the actual reason, which might reveal the reviewer's own misunderstanding instead.

Checklist: giving feedback that lands well

1. Explicitly label every comment as blocking or non-blocking (nit) — never leave the author to guess.
2. Critique the code, never the person — phrase every comment about what the code does, never about the author's competence or diligence.
3. Be specific: name the exact concern, the exact location, and (where possible) a concrete suggested direction — avoid vague, unlocated observations.
4. Default to assuming positive intent — ask, don't accuse — even for what looks like a straightforward mistake, a question produces the same correction with none of the relational cost.

Discussion / interview questions

1. "Why is it important to explicitly label a review comment as blocking versus a nit?" Strong answer: Without explicit labeling, the author has to guess which comments are mandatory and which are optional — leading to either wasted time over-correcting non-issues or missed fixes from under-correcting something meant to be blocking; explicit labeling respects the author's time and removes this ambiguity entirely.

2. "Rewrite this review comment to be more effective: 'This is wrong, you didn't handle errors properly.'" Strong answer: Produces something like: "BLOCKING: This doesn't handle the case where the API call fails — right now an exception here would propagate uncaught. Consider wrapping this in a try/catch and logging the failure, per our error handling standards." — specific, depersonalized, and actionable.

3. "What does 'assume positive intent' mean in the context of giving code review feedback, and why does it matter beyond just being polite?" Strong answer: Assume the author had a reason for their choice and phrase feedback as a question rather than an accusation — this is often more accurate (the author may have context the reviewer lacks) and produces the same corrective outcome as an accusatory comment would, without the relational damage, even when the author did make a genuine mistake.

4. "A reviewer leaves the comment: 'There might be edge cases you're missing here.' What's wrong with this feedback, and how would you improve it?" Strong answer: It's too vague to act on — it doesn't identify which specific edge case, or where; an improved version names the specific scenario the reviewer is concerned about (e.g., "what happens if this array is empty?") so the author doesn't have to independently rediscover what the reviewer already found.

Chapter summary / cheat sheet

• Explicitly label every comment as blocking or a non-blocking nit — never leave the author guessing.
• The same technical concern can be phrased destructively (accusatory, vague) or constructively (specific, depersonalized, actionable) — the substance doesn't need to change, only the phrasing.
• Assume positive intent by default: ask rather than accuse, even for an apparent straightforward mistake — this is often more accurate and produces the same correction with no relational cost.
• Be specific: name the exact concern and location rather than a vague, unlocated observation, so the author doesn't have to redo the reviewer's diagnostic work.

---