1.7 KiB
1.7 KiB
description
| description |
|---|
| Review a diff or merge request for real bugs and missed requirements |
Review the change for real problems. Do not edit anything.
Target: $ARGUMENTS
When the target names a merge request or pull request, review that change. When it names a file or directory, review what is uncommitted there. When it is empty, review the current working diff.
How to review:
- Read the change, then read the code around it. Context you did not read is context you will misjudge.
- Look for real problems only: logic that is wrong, a case that was missed, a contract broken, data lost, an error swallowed, a race introduced, something the change claims to do that it does not do.
- Check the change against whatever guidance governs the code it touches. Guidance is guidance for writing code, so not all of it applies to a review.
- Read the git history of the lines being modified. A change that looks fine in isolation often breaks a reason the code is the way it is.
- Leave these out: style, naming, formatting, import order, missing tests, missing documentation, and anything a linter, typechecker, or compiler will catch. CI runs those. Also leave out problems on lines the change did not touch.
Output:
- Order findings by severity, highest first.
- Each finding gets the file and line, what is wrong, and why it matters. Link to code rather than quoting long stretches of it.
- Say which requirement or guideline is broken when that is the reason.
- When a finding is a judgement call rather than a definite bug, say so plainly instead of dressing it up as certain.
- If there is nothing worth reporting, say that and stop. Do not invent findings to fill the space.