Skip to main content

Writing Review Comments: Specific, Constructive, Evidence-Based

What This Concept Is

A review comment is a short written artifact with an unusually high effort-to-effect ratio. Good ones change designs. Bad ones create churn, hurt feelings, and get ignored. A good comment is:

  • Specific -- about a named line, symbol, or behavior, not "this feels off."
  • Constructive -- offers either a concrete alternative or a concrete question.
  • Evidence-based -- references the codebase, a test, a spec, or a reproducible scenario.

Google's "How to Write Code Review Comments" adds: be kind, explain your reasoning, and distinguish things you require from things you suggest.

Why It Matters Here

Review comments are the primary place where engineering judgment becomes visible. They shape how teammates see you (sharp and helpful, or pedantic and slow). They also shape the codebase, because a comment accepted once becomes the precedent for later PRs. If your comments are vague or absolute, they start shaping the wrong things.

Concrete Example

A weak comment:

"This is messy."

Five things wrong with it: not specific, not constructive, not evidence-based, not labeled (nit or substantive?), not explained.

A good comment on the same spot:

[clarity, nit] The nested ternary on line 57 is hard to read. Consider an early-return guard:

if not user.is_active:
return default_permissions
if user.role == "admin":
return admin_permissions
return user.custom_permissions

Not blocking -- up to you.

This version names the line, offers a concrete alternative, labels it, and explicitly says the author can override.

Another good comment, this one blocking:

[correctness, substantive] charge() returns None when the gateway returns HTTP 503, so the caller silently records a successful charge. Reproduce with the fixture tests/fixtures/gateway_503.json. This needs to raise TransientGatewayError or return a typed failure before merge.

Common Confusion / Misconception

"Nice" comments are not the same as kind comments. Kindness is useful; performative niceness ("great idea! just wondering, maybe could we possibly...") confuses the author about whether a change is required. Be warm and clear.

"I would have done it differently" is not a comment. Preference is not evidence. Either tie the preference to the rubric (correctness / clarity / design / tests) or let it go.

"Why?" questions can be passive-aggressive. "Why did you do this?" without context reads as accusation. Reframe as "I may be missing context -- this read as X to me. Is that intended?"

Absolute phrasing damages reviews. "You never handle..." / "This is always wrong..." is rarely true and always fighting words. Describe the specific case.

Nit-flooding. Ten nits on one file drown out two real issues. Pick the two biggest nits, or roll them up: "Several nested ternaries in this file -- consider a formatter rule."

How To Use It

A template that scales well:

[bucket, nit|substantive] Observation tied to a specific location. Evidence (test, spec, repro). Concrete suggestion or concrete question. Whether blocking.

Examples by bucket:

  • Correctness: "Line 112 double-counts tax when discount=0; see test test_zero_discount. Please fix or add a failing test first."
  • Clarity: "doFinalCheck is the third method in this file with a vague name. Rename to rejectIfFraudulent?"
  • Design: "This is the second caller constructing PricingStrategy via if chain. We already have PricingFactory -- route through it to keep one place that changes when we add a new strategy."
  • Tests: "No test covers the retry branch; without one, next month's refactor will break this silently."

Habits that compound:

  • Read the whole PR before commenting.
  • Group comments by file and by bucket, not by scroll order.
  • End with a short summary: "Overall: looks good; please address the two [correctness, substantive] items and ship."

Check Yourself

  1. Why is labeling a comment "nit" respectful to the author?
  2. What is the minimum evidence a [correctness, substantive] comment should include?
  3. Give a rephrasing of "this is wrong" that preserves the concern but invites discussion.

Mini Drill or Application

Rewrite each weak comment into a good one, adding a bucket label, a location, evidence, and a concrete suggestion:

  1. "I don't like this function."
  2. "Why didn't you use Strategy?"
  3. "Tests?"
  4. "This looks slow."
  5. "Didn't we have a pattern for this?"

Read This Only If Stuck