Code review is a craft. This guide covers what separates a useful review from noise — from spotting real issues to the tone that keeps teams moving.
Read the entire diff before writing a single comment. You need the full picture to know what matters. A change that looks suspicious in one file might be intentional given what happens in another.
Not all observations are worth a comment. Focus on things that actually matter: bugs, security holes, logic errors, missing edge cases, misleading names, and violations of patterns the codebase already establishes.
Correctness
Will this code do what the author thinks it does?
Security
Does this open an injection vector, leak data, or skip auth?
Performance
N+1 queries, unbounded loops, unnecessary allocations
API / contract
Breaking changes, bad error handling, undocumented behaviour
Naming & clarity
Names that mislead, dead code left in, missing context
A vague comment forces the author to guess what you mean and come back with follow-up questions. State what the problem is, why it matters, and what to do about it.
Weak
"This doesn't look right."
Strong
This will panic on an empty slice — the loop starts at index 1 but doesn't guard against len(items) == 0. Add a length check before the loop.
If the author has to read between the lines to know whether your comment blocks the merge, you've made their job harder. Label every comment so they can triage at a glance.
"This bypasses the auth middleware — ship this and anyone can hit the admin endpoint."
"Fetching inside a loop will cause N+1 queries on every page load. Worth batching."
"nit: `getUserById` → `findUser` is closer to the naming convention in this file."
More words do not mean more clarity. Long comments that over-explain bury the actual point. Aim for the fewest words that make your meaning impossible to misread.
Over-explained
"So I was looking at this and thinking about it and I'm not entirely sure but it seems like maybe there could potentially be an issue here with the error handling, though I could be wrong, just thought it might be worth looking into?"
To the point
The error from db.Query() is swallowed here — if the query fails, the caller gets nil data with no indication of why.
When the fix is obvious and short, write it out. Pointing at a problem without showing the solution adds friction. Showing the fix closes the loop in one round-trip.
Anchoring a comment to a single line works for single-line issues. But when the problem spans a function, a block, or a whole section — select the whole range. It gives the author the right context immediately.
When to use single-line
Typo in a variable name, off-by-one on a single expression, a missing null check on one return value.
When to use multi-line
The entire function has the wrong approach, a transaction isn't closed properly across several lines, or the issue only makes sense when you see the setup and teardown together.
Inline comments are details. The verdict is the bottom line. Always close a review with a PR-level summary that tells the author where things stand — don't make them count up blockers to figure it out.
Approve
The code is good to merge. Inline nits are optional — call them out so the author knows they're not blockers.
Request changes
There are one or more issues that should be addressed before merge. List them clearly at the top of your summary.
Comment (no verdict)
You have questions or thoughts but aren't blocking. Make that explicit — 'not blocking, just curious about the approach here.'
Good review judgement is situational. The same nit that's worth raising in a calm sprint might not be worth raising when a hotfix needs to be live in 20 minutes.
You and the author are on the same team. The code is the thing under review — not the person. A review that feels like an attack makes people defensive, and defensive people don't absorb feedback.
Combative
"Why would you do it this way? This is clearly wrong. You should have known better."
Collaborative
"This will silently return stale data when the cache expires — we ran into this in the users service last quarter. Adding a TTL check here should fix it."
The best reviewers make their team better over time — not just this PR, but the next one too. People improve faster when feedback feels safe to receive.
Ready to put this into practice?
rate_reviewBrowse Challenges