sebastienrousseau/agtmls/giving-code-review

Giving code review

Trigger. You are reading a change you did not write and are expected to respond: a pull request, a patch, a diff, a colleague asking "does this look right?".

Reference material (the correctness pass in detail, severity calibration,

worked comment rewrites, and reviewing agent-authored changes): see

reference.md in this directory.

The rule

**Every comment states its severity. A reviewer who does not distinguish

"this is broken" from "I'd have named it differently" makes both easy to

ignore.**

Unlabelled feedback forces the author to guess what blocks the merge, and they guess wrong in both directions — polishing nits while shipping the bug, or dismissing a real defect as taste.

Read in this order

Reading a diff top-to-bottom is how correctness bugs get missed while indentation collects three comments. Work outward instead:

1. Does it do the right thing? Read the description and the tests before

the implementation. If you cannot tell what the change is for, that is your first comment.

1. Is it correct? Trace the logic — boundaries, error paths, concurrency,

the empty case and the enormous one.

1. Will it hold up? Security, performance at real scale, failure modes,

backward compatibility.

1. Is it maintainable? Naming, structure, duplication, comments that will

go stale.

1. Is it consistent? House style and formatting. Last, and usually a

linter's job rather than a human's.

Run out of time and you have still covered the levels that matter.

Label every comment

Two rules follow:

• Be honest about the ratio. If everything is blocking, nothing is. Most

comments on a competent change are nit or consider.

• `question` must be a real question. "Why would you do it this way?" is

criticism in costume — say the criticism instead.

Make comments actionable

A comment the author cannot act on is noise. Three parts: what, why, what instead.

Point at the line, say what breaks, offer a direction. You do not owe a full solution, but "this is wrong, figure it out" is not review.

Proportion

Match depth to stakes. A config tweak and a new auth path do not deserve the same scrutiny, and treating them alike trains people to ignore you.

• Do not redesign in review. If the approach is wrong, say so once, early,

at the top level — not through twenty inline comments implementing your version.

• Do not block on preference. Correct, tested, and readable means your

taste is not a merge condition.

• Do not review the author. Comment on the code.
• Approve when it is good enough, not when it is what you would have

written. Perfect is a way of never shipping.

When not to use

• You wrote the change — that is receiving-code-review.
• The project defines its own review protocol or approval rules; follow it.
• You are auditing for a specific risk class rather than reviewing a change.