# Code Review Checklist

**Change:** _______________  **Reviewer:** _______  **Date:** _______

> Order matters. Style comments are cheap and satisfying, and a review that opens with them
> rarely reaches the questions that prevent incidents. Work down the list.

## 1. Before reading the code

- [ ] **What problem does this solve?** If the change does not say, ask before reviewing
- [ ] Is this the right problem to solve now?
- [ ] Is the change small enough to review properly? **If not, say so and stop** — a 900-line
      review is a rubber stamp with extra steps
- [ ] Does the description explain the approach, not just restate the diff?

## 2. Correctness where it is usually wrong

- [ ] Empty input, single item, maximum size
- [ ] Null, missing, and explicitly-set-to-nothing treated distinctly where they differ
- [ ] Off-by-one at every boundary
- [ ] **Concurrent execution** — can two of these run at once, and what happens?
- [ ] Time zones, daylight saving, and dates near month and year boundaries
- [ ] Numeric precision where money is involved
- [ ] Encoding assumptions on anything that came from outside

## 3. Failure behaviour

- [ ] **What happens when the thing it calls fails?** Named, not assumed
- [ ] Timeouts set on every external call
- [ ] Retries only where a retry can succeed, and with backoff
- [ ] **Retry safety** — is the operation idempotent, or can it be repeated harmfully?
- [ ] Partial failure leaves a consistent state
- [ ] Errors reach somewhere a person will see them
- [ ] Failure messages say what to do, not only what went wrong

> "What happens when this fails?" is the highest-yield question in code review. Ask it about
> every call that leaves the process.

## 4. Reversibility

- [ ] Can this be rolled back? If not, that is a design decision — is it a deliberate one?
- [ ] **Database changes are backward-compatible** with the currently deployed code
- [ ] Behind a flag if it is risky, and the flag has a removal plan
- [ ] Data migrations can be re-run safely

## 5. Security, proportionate to what it touches

- [ ] Input from outside is validated at the boundary
- [ ] **Authorisation checked where the data is fetched**, not in the caller
- [ ] Queries parameterised
- [ ] Output encoded for its destination
- [ ] No secret in code, configuration file, log line or error message
- [ ] Logs do not contain personal data that will outlive its purpose

## 6. Tests

- [ ] **Would the tests fail if the code were wrong?** Check one deliberately
- [ ] The specific bug being fixed has a test that fails without the fix
- [ ] Boundaries and failure paths tested, not only the happy path
- [ ] Tests assert on behaviour, not on internals that will change on the next refactor
- [ ] No new flaky test — if it needs a fixed wait, it is not finished

## 7. Change to existing behaviour

- [ ] Anything depending on the old behaviour has been identified
- [ ] Interfaces others consume are versioned or unchanged
- [ ] Output shape unchanged, or every consumer updated
- [ ] Removed code is genuinely unused — checked, not assumed

## 8. Fit

- [ ] Consistent with how this codebase already does things
- [ ] **No new pattern introduced without a stated reason** — one codebase with three
      approaches to the same problem is more expensive than any of them
- [ ] No dependency added without a reason and a look at what it pulls in
- [ ] Abstraction earns its place — one caller does not need an interface

## 9. Readability, last

- [ ] Names say what the thing is
- [ ] Comments explain **why**, never what
- [ ] Nothing surprising left unremarked
- [ ] A stranger could follow the main path in one read

## 10. As the author, before requesting review

- [ ] Read the diff yourself first
- [ ] Removed debugging output, commented-out code and unrelated changes
- [ ] Said what you are unsure about — it directs the reviewer to where they are most useful

## Sign-off

| | Name | Date |
|---|---|---|
| Author | | |
| Reviewer | | |