Review comments that land

The same concern can read as a gift or an attack — the wording decides which.

The idea

A review comment has one job: get a good change made without bruising the person who made it. The shape that works is observation, then concrete impact, then a question or a suggested fix — and a severity label so the author knows whether to drop everything or file it under later. Describe the code, not the coder; say what it costs, not just that you dislike it; and hold the merge only when the concern actually earns it.

Assemble a comment below and watch the “how it lands” gauge move. Flip between a security bug and a style nit to see when blocking is right — and when it just slows the team.

How it works

Four moves turn a reaction into a comment an author will act on:

1. OBSERVE   describe the code, not the person
             "I noticed this concatenates name into the SQL string"
             (not "you wrote this unsafely")

2. IMPACT    say what it costs, concretely
             "that opens a SQL-injection path"
             (not "this is bad" — and not "we'll get hacked, disaster!")

3. ASK       a question or a suggested fix beats an order
             "could we use a parameterized query — db.query('… ?', [name])?"

4. LABEL     blocking | suggestion | nit   -> so the author can triage
             block only correctness, security, or data loss
             everything else is a suggestion or a nit that can merge

Pushing back, including on someone senior: anchor on the code and its impact, not authority. “Help me understand the trade-off — with untrusted input, doesn’t this concat allow injection?” invites a technical answer and gives them room to reconsider without losing face. Ask, cite the concrete risk, and stay open to being wrong — seniority is not the argument, the injection path is.

When to use it

Watch out for

• “You” framing. “You broke the query” puts the author on defense. Describe the code — “this query” — and the fix gets read instead of resisted.
• Unlabeled severity. Without a label the author can’t tell a merge-blocker from a preference, so they either over-react or ignore you. Always tag it.
• Blocking on taste. Holding a merge over naming or a style you’d have done differently burns trust and velocity. Reserve the block for correctness, security, and data loss.
• No path forward. “Rewrite this” leaves the author guessing. A concrete suggestion — even a one-line snippet — converts far more often.
• Catastrophizing. “This will get us hacked, total disaster” reads as drama and gets discounted. State the specific, real consequence and let it carry the weight.

Worked example

An interviewer asks, “A senior engineer’s PR concatenates user input into a SQL query. How do you comment?” The weak answer blocks with “this is insecure, fix it.” The strong answer is a full comment: “blocking (security): I noticed this builds the query by concatenating name into the SQL string. With untrusted input that opens a SQL-injection path — a crafted name could read or drop rows. Could we switch to a parameterized query, e.g. db.query('… name = ?', [name])? Happy to pair if useful.” It observes, names the concrete impact, offers a fix, labels the severity honestly, and — because it is a security defect, not a preference — holds the merge. Being junior is irrelevant; the injection path is the argument.