Skip to content

From the team

Code review practices that actually catch bugs

Small pull requests, automated checks before a human looks, a short checklist and a culture that treats review as part of the job. What good code review looks like.

5 min read

Code review is the practice of having a second developer read a change before it is merged into the product. Done well, it is the cheapest bug-catching step in the whole delivery process, cheaper than testing and far cheaper than a production incident. Done badly, it is a rubber stamp that adds a day of delay and catches nothing. This article is for engineering leads and developers who want their reviews to find real problems, and for business owners and product managers who want to know what to ask for when they are told "we do code review".

Why do most code reviews miss bugs?

Because the reviewer is looking at too much, too late, with no idea what to look for.

A large change arrives at the end of a sprint. They skim it, comment on a variable name and a missing comma, and approve. The logic error in the middle of it goes unnoticed because reading a thousand lines carefully takes hours nobody has budgeted.

Every practice below exists to remove one of those conditions: the size, the timing, the lack of focus, or the lack of a shared standard for what "reviewed" means.

How small should a pull request be?

Small enough to be reviewed properly in the time a reviewer can actually give it. In practice that means one logical change per pull request: one feature, one fix, one refactor.

Small pull requests have consequences beyond the review itself:

  • They merge faster, so work does not pile up unmerged and conflict with itself.
  • They are easier to revert if something goes wrong in production.
  • They make a slow reviewer visible, whereas a large one hides the delay in the size.

The usual objection is that some features are simply big. They are, and the answer is to break them into a sequence of changes that each leave the product working: add the database column, then the backend endpoint, then the screen.

What should automated checks handle before a human looks?

Anything a machine can decide, a machine should decide, before a person spends attention on it. A reviewer who is pointing out formatting or an unused import is doing work that should have been finished before the review opened.

The minimum set that runs automatically on every pull request:

  1. Formatting and linting, so style is never discussed by humans.
  2. Static analysis, catching type errors, unreachable code and obvious mistakes.
  3. The test suite, with the rule that a red build is not reviewed until it is green.
  4. Security and dependency scanning, flagging known vulnerabilities in what was added.
  5. A build of the actual application, so "it works on my machine" is not a review comment.

With these in place, the reviewer's attention goes entirely to what a machine cannot judge.

What should a reviewer actually look for?

A checklist, kept short and used every time, beats a reviewer's instinct on a busy afternoon. Ours is a set of questions, and the order matters:

  • Does it do what the description says? Read the description first, then the code, and check that they match.
  • What happens when it fails? Every external call, every user input and every database write can fail. Look for the branch that handles it, or the absence of one.
  • Could this be wrong with different data? Empty lists, very long text, Arabic text and right-to-left layout, a user with no permissions, the same request sent twice.
  • Does it change behaviour anywhere it should not? A changed shared function has callers the author may not have thought about.
  • Is it tested for the thing that matters? Not whether tests exist, but whether they would fail if the logic were wrong.
  • Would I understand this in six months? Naming, structure and comments where the reason is not obvious.

What a reviewer should not do is redesign the change to how they would have written it. That is a conversation to have before the work started, not in the review.

How do you build a review culture that people do not resent?

Review is a social practice as much as a technical one, and it fails socially before it fails technically.

Comments should be about the code, never about the person, and it should be clear which comments block the merge and which are suggestions.

Turnaround matters. A review that waits two days is a review that has already cost more than the bug it might catch. Reviewing other people's work is part of the job, not an interruption to it, and it goes in the plan for each two-week iteration alongside the features.

Authors should review too, including senior ones, and everyone's work gets reviewed, including the lead's. The moment review becomes something juniors receive and seniors bypass, it stops being about quality.

Finally, review should be visible to the client. On our projects the client portal shows what is in progress, in review and done, so review is a stage of delivery rather than an invisible delay, and the demo at the end of each iteration shows work that has already been through it.