Code review language
Code review is where engineers criticize each other's work daily, in writing, in public, forever. Unsurprisingly, it grew a specialized dialect whose whole purpose is calibration: making it cheap to say "this is fine, one tiny thing" and unmistakable to say "this must not merge." Learn the dialect and reviews get faster and friendlier; ignore it and every comment carries unintended weight.
Severity labels: nit, blocking, optional
The core convention is prefixing comments with a severity label:
| Label | Meaning |
|---|---|
| nit: | Nitpick. Trivial; fix or ignore as you like. "nit: extra blank line." |
| optional: / take it or leave it: | A real suggestion, author's call. |
| suggestion: | Concrete improvement, mildly held. |
| question: | Genuine question, not a disguised demand. |
| issue: / blocking: | Must be resolved before merge. |
| non-blocking: | Explicitly does not hold up merge, even if substantive. |
| praise: | Calling out something good. Underused everywhere. |
Some teams adopt this formally (the "Conventional Comments" standard adds
labels like thought: and chore:); most use it informally. The value is
the same: the author can triage twenty comments instantly, and reviewers can
be picky about small things without those small things reading as demands.
Unlabeled comments default to an ambiguous middle weight, which is exactly
where misunderstandings breed. When in doubt, label.
Praise deserves its own sentence: reviews that only ever surface problems train authors to dread them. "praise: this test table is really readable" is one line and changes the whole temperature of a review.
Phrasing comments: requests, not commands
The mechanical rules that keep review comments from sounding like orders:
- Prefer questions and suggestions to imperatives. "What do you think about extracting this into a helper?" or "Consider extracting this" rather than "Extract this." The question form also leaves room for the author to know something you do not, which is often the case.
- Make the code the subject, not the person. "This function also handles retries; could we split the concerns?" rather than "You made this function do two things." The author and the code are different entities; review the code. (This is Chapter 5's artifact rule, applied line by line.)
- Give the why, compressed. "suggestion: use a set here; this list scan is O(n) per event and this path is hot" teaches; "use a set" merely instructs. One clause of rationale is usually enough.
- Anchor claims when you can. "This will NPE when config is absent; see the loader at startup.py:88" is a gift. Vague unease ("something feels off here") is allowed but say it as that: "thought: no concrete objection, but this coupling makes me nervous. Fine to merge if you've considered it."
The approval vocabulary
- LGTM: "looks good to me", the classic approval.
- LGTM with nits: approved; fix the trivia, no re-review needed.
- Approving to unblock; please address the two comments before merging. Trust-based approval, common between senior peers and across time zones.
- PTAL: "please take another look", said by the author after addressing feedback.
- Requesting changes (the formal review state): reserve it for genuine blockers, and say which comments are the blocking ones. A "request changes" over pure nits reads as disproportionate; a wall of 30 comments where none is marked blocking is unreadable.
Don't be confused: LGTM approves the change; +1 merely agrees with a statement or adds support ("+1 to Sam's concern" means "I share that concern", the opposite of approval). ACK means "acknowledged, received", and WIP on a PR title means "work in progress, don't review seriously yet" (many teams use draft PRs for the same signal). None of these are interchangeable, and using LGTM to mean "I skimmed it" devalues the token; only say it when you mean "I reviewed this and would defend merging it."
Responding to review, as the author
The author's half of the dialect matters just as much:
- Answer every comment, even trivially: "Done." / "Fixed in a3f9c21." / "Good catch, fixed." Silently resolving comments makes reviewers re-audit everything; the one-word replies are the receipt.
- Push back openly when you disagree, using the ladder from Chapter 5: "I'd prefer to keep it inline: the helper would have one caller and this function is already short. Happy to extract it if you feel strongly." Then let the severity label govern: if their comment was optional and you declined with a reason, resolve and move on.
- Escalate scope honestly. When a comment reveals a genuinely bigger problem: "You're right and it's structural; fixing it properly touches the whole module. I've filed #482 and would rather land this as-is behind the flag. OK?" Deferring with a tracked ticket is professional; deferring into the void is how debt becomes invisible.
- Thank the reviewer when the review was substantial. One line. See Chapter 2.
Asking for review
The request itself has etiquette:
- Size the ask: "Small one, ~60 lines, mostly mechanical: PTAL when convenient." versus "This is the big migration PR. It needs a careful read; 30 to 45 minutes. Could you book time this week?"
- State urgency honestly and rarely: "This blocks the release cut Thursday" when true; nothing is urgent every week.
- Route review well: one named reviewer beats a broadcast. "@Sam for the storage changes, @Ines for the API surface" beats hoping someone bites.
- If review is slow, bump per Chapter 3: gently, once, then re-route: "Sam's out; Ravi, could you take this one?"
Review disagreements that outgrow the PR
Some review threads stop being about the PR: a ten-comment argument about error-handling philosophy is no longer review, it is design discussion in the wrong venue. The standard move:
This thread is bigger than this PR. I've opened #491 to discuss our
error-handling convention properly; proposing we land this PR following
the existing pattern and apply whatever we decide there across the module.
That sentence structure (name the mismatch of venue, open the right venue, propose a default so nothing stalls) resolves ninety percent of stuck threads. For the other ten percent, get synchronous: "We're going in circles in text; fifteen minutes on a call?" Text escalates tone by accident; voice de-escalates it by default.
👉 Reviews happen when code is ready; standups and status updates happen on a clock, whether anything is ready or not. Next: the language of the sprint, namely how to report progress in standup without rambling, the exact difference between "on track", "at risk", and "blocked", and how to write an async status update people actually read. On to Chapter 7.