Review Request Language
Requesting reviews, labelling comments, constructive feedback, review states, and response language
Review request and response essentials
- Request formula: @person + context + focus area + realistic timeline — never "ASAP"
- Comment labels: "nit:", "suggestion:", "optional:", "blocking:" — be explicit about priority
- Constructive comments: specific problem + specific solution + concrete example or link
- Review states: Approve = ready to merge; Comment = FYI/no block; Request changes = must fix
- Responses: "Fixed in auth/tokens.go:L34 — reusing the existing error type" beats "Fixed."
Question 0 of 5
Which review request comment is most effective when asking a colleague to review your PR?
Specific person + context + focus area + timeline. Review request formula:
- Specific person: "@sarah" — not a broadcast to the whole team
- Context: "auth fix (#892)" — they can prep before opening the PR
- Focus area: "session refresh logic in middleware/auth.go:L45–L78" — saves 20 minutes of orientation
- Timeline: "EOD tomorrow" — respectful, doesn't demand immediate attention
How should you label a review comment to indicate it is not a blocking issue?
"nit:", "optional:", "suggestion:" — label the priority of each comment explicitly. Review comment priority labels:
- blocking / must-fix: "This will cause a data race under concurrent requests — must fix before merge"
- nit: "nit: variable name could be more descriptive — trivial, your call"
- suggestion: "suggestion: consider extracting this into a helper for readability — not a blocker"
- optional: "optional: we could add a cache here but it's not needed for v1"
- question: "question: why did we choose Redis here vs. in-memory cache?"
Which code review comment is written most constructively?
Specific problem + specific solution + concrete example. Constructive review comment formula:
- Problem: "This function will time out for large datasets" — specific, not "this is wrong"
- Solution: "add pagination (limit/offset) or cursor-based approach" — actionable
- Example: "Here's how users service does it: [link]" — concrete reference
When should you use "Request changes" vs. "Comment" vs. "Approve" on a GitHub PR review?
Request changes = must fix; Comment = FYI/discussion; Approve = ready to merge. GitHub review states:
- Approve: "I've reviewed this thoroughly and it's ready to merge. Small nits are at the author's discretion."
- Comment: "I have questions or suggestions but I'm not blocking the merge — use your judgement."
- Request changes: "There are issues here that must be resolved before this merges. I'll re-review after changes."
- ❌ Approving then leaving 10 blocking comments — the author will merge ignoring the comments
- ❌ Requesting changes for nits — holds up the PR unnecessarily
- ❌ Always commenting (never approving) — leaves authors unsure if they can merge
An author responds to a review comment: "Fixed." Why is this insufficient?
"Fixed" gives the reviewer no context for reverification. How to respond to review comments:
- ❌ "Fixed." — reviewer must re-read the entire diff to find what changed
- ✅ "Fixed — extracted the validation logic into validateSessionToken() in auth/tokens.go:L34. Reusing the existing error type from pkg/errors."
- ✅ "Good catch — refactored to use cursor-based pagination (see users/service.go:L200–L240). Added a test covering the 10K+ record case."