Skip to content
~/ashwin-shresthaLoading

000%

All posts

A code review checklist for a small team

What to look at, in what order, and how to give feedback that gets merged instead of argued about. A practical checklist for teams of around five.

Code review is the cheapest quality tool a team has, and also the easiest to do badly. Reviews that only check formatting miss real bugs. Reviews that nitpick everything slow the team down and make people dread opening a pull request. A short, ordered checklist fixes both problems.

Before you read a line of code

Is the pull request reviewable? A change that touches thirty files and three unrelated concerns cannot be reviewed well. Aim for one purpose per pull request and a description that answers: what changed, why, and how to test it. If you cannot tell what it is for from the title, ask before reading further.

Review in this order

1. Does it solve the right problem?

Compare the change with the ticket or requirement. A beautifully written solution to the wrong problem is the most expensive kind of waste, and it is cheapest to catch first.

2. Is it correct?

Look for the failure cases, not the happy path.

  • What happens with empty, null or very large input?
  • Are errors handled, or silently swallowed?
  • Could two requests run at once and corrupt state? Money and inventory deserve extra care.
  • Are new queries inside loops? That is how one-second pages become ten-second pages.

3. Is it safe?

  • Is every input validated on the server, not only in the form?
  • Are authorisation checks present on the route, not just hidden in the UI?
  • Are secrets, tokens or personal data being logged or committed?
  • Do database migrations run safely on a table that already has data, and can they be rolled back?

4. Is it tested?

Ask what would fail if this change were reverted. If the answer is "nothing", there is no test for it. Tests should cover the unhappy path and the bug that prompted the change.

5. Can the next person understand it?

Names that say what things are, functions that do one thing, and comments that explain why rather than what. If you had to ask the author how something works, the code (or a comment) needs to say it instead.

6. Style and formatting last

Let tools do this. A formatter and linter in the pipeline remove an entire category of review comments, so humans can spend their attention on logic.

Giving feedback

  • Ask, do not accuse. "What happens if the list is empty?" gets a better result than "You forgot the empty case."
  • Label severity. Mark each comment as blocking, a suggestion, or a nit. Authors should never have to guess which comments can wait.
  • Explain the reason. "Move this out of the loop because it runs one query per row" teaches; "move this" does not.
  • Praise the good parts. Pointing out a clean solution is how a team learns what good looks like.
  • Do not rewrite the change in comments. If you would solve it very differently, talk to the author or pair for ten minutes.

Receiving feedback

Treat comments as being about the code, not about you. Reply to every comment, even if only with "done". If you disagree, say why and propose an alternative; if you still disagree, bring in a third person rather than going back and forth.

Keeping it fast

Slow reviews are a hidden tax on every developer. Agree a target, such as a first response within one working day, and make small pull requests the norm so that target is realistic. For urgent fixes, a quick pair review beats skipping review altogether.

The checklist, condensed

  1. Right problem, small and clearly described change.
  2. Correct, including failure cases and concurrency.
  3. Safe: validation, authorisation, secrets, migrations.
  4. Tested, including the unhappy path.
  5. Readable by someone who was not there.
  6. Formatting handled by tools.
  7. Feedback labelled, specific and kind.

A team that follows even half of this ends up with a main branch that is always releasable, and that is worth more than any single clever feature.