Code Review Won't Find Many Bugs. Do It Anyway.

A note before you start (updated September 2026). The talk this is based on was last given in 2023. That was before AI-assisted coding became part of most developers' day. Some of the advice assumes a human wrote every line being reviewed, and that's no longer a safe assumption. That said, a surprising amount of it is still relevant, and some of it matters more now than it did then. I'll write about code review in the age of AI separately.

If you're trying to convince your manager to introduce code review, you'll probably lead with bugs. "We'll catch bugs before they reach production."

Don't. You'll be setting yourself up to fail.

Studies of code review consistently find that only around a fifth to a quarter of the issues it uncovers are what we'd traditionally call bugs. If you promise a big drop in bug counts, you probably won't deliver it, and code review will look like it isn't working.

It is working. It's just working on something more valuable.

Two kinds of defect

It helps to split defects into two groups.

Bugs. Code that crashes, or gives the wrong result.

Evolvability defects. Code that makes the codebase less compliant with standards, more error prone, or more difficult to modify, extend or understand. That's the formal definition from the research. It's a posh phrase for technical debt.

Most of what code review finds falls into the second group. Research by Mäntylä and Lassenius, and separately by Siy and Votta, puts evolvability defects at roughly three quarters or more of review findings.

That might sound disappointing. It isn't, because evolvability defects are expensive. One study found that on code with low evolvability, new features took 28% longer to implement and bug fixes took 36% longer. Another estimated that software structure may account for around a quarter of total maintenance costs.

Think about what that means over the life of a project. On a codebase that stays clean, a feature of a given size costs about the same in year three as it did in month three. On a codebase full of evolvability defects, the same sized feature costs more and more every year.

That's the pitch. Effective code review reduces the overall cost of software development. Not mainly by catching bugs, but by stopping the codebase getting harder to work with.

And the same principle applies to all defects: the later you find them, the more they cost. A problem spotted in review costs a comment and a few minutes of rework. The same problem found in production, or two years later when someone tries to extend the code, costs far more.

The other benefits

Fewer defects is the headline, but there are three more benefits worth knowing about.

Security. A second pair of eyes is good at spotting things like sensitive data being written to logs. (PHP 8.2's #[SensitiveParameter] attribute helps here, by keeping sensitive arguments out of stack traces.) Reviewers can also spot files that shouldn't be in the change at all, and anyone rolling their own authentication, hashing or encryption. Keep the OWASP Top 10 and its cheat sheets to hand.

Spreading knowledge. Every project should worry about its bus factor: how many people would need to be hit by a bus (or, more cheerfully, win the lottery and move to a beach) before the project is in trouble. If only one person understands the payments code, your bus factor is one. Code review means at least two people have seen every change.

Mentoring. Reviewing and being reviewed are both great ways to learn. More on that later.

Automate first

Before any human looks at a change, everything that can be automated should be.

Tests: PHPUnit, Behat, Codeception. Coding standards: PHP_CodeSniffer or PHP CS Fixer. Static analysis: PHPStan or Psalm. All of it running in CI: GitHub Actions, Jenkins, CircleCI, whatever you use.

The rule is simple. Humans only review what machines can't.

Computer time is cheap. Human time is expensive. No human should ever leave a review comment about brace placement or a missing type. If a tool can decide it, a tool should decide it, and the change shouldn't reach a reviewer until CI is green.

That frees the reviewer to focus on the things only a human can judge.

Review in the right order

When I pick up a review, I work through it in three stages.

1. Understand the purpose

What is this change supposed to do? Read the ticket, the story, or the pull request description. You can't judge whether code is right if you don't know what it's for.

2. Review the tests

This is where most of the bugs are found. Not by reading the implementation, but by checking that the tests encode the requirements correctly.

Here's an example. The requirement is a slug generator: output must be lowercase, and anything that isn't a letter or number becomes a dash. The tests check these inputs and expected outputs:

• my blog → my-blog
• hello Dave → hello-Dave

The tests pass. CI is green. But the second test is wrong. The requirement says lowercase, so the expected output should be hello-dave. The implementation is doing exactly what the test asked for, and the test asked for the wrong thing.

The reviewer spotted a bug without reading a single line of the implementation, which was probably a regular expression that would have taken far longer to understand.

The next question is whether all the cases are covered. What about multiple spaces? Apostrophes?

• my blog (two spaces) → my-blog
• it's Saturday → its-saturday

If they're missing, ask for them. You might have found a bug. You might not. Either way, the behaviour should be tested.

A handy rule of thumb: look at the method signature for the minimum number of tests. A method that returns a bool and can throw an exception needs at least three: the true path, the false path, and the exception. That's before you think about any business cases.

If the tests are testing the right things, and they pass, the implementation is probably OK.

3. Review the rest of the code, commit by commit

Now look at the implementation. This is mostly where you'll find evolvability defects, with static analysis having already caught many of the bugs a human might otherwise miss.

What to look for

Here's the checklist I use.

Will I understand this code in six months?

The reviewer isn't in the author's head. That makes them a good stand-in for whoever reads this code next year, which might be the author, having forgotten everything.

We spend far more time reading code than writing it. Optimise for reading.

Here's a real example from a review:

$userFields = ['Username', 'Email', 'FirstName', 'LastName', 'Phone'];

foreach ($userFields as $key) {
    if ($userDetails->{'get' . $key}()) {
        $user->{'set' . $key}($userDetails->{'get' . $key}());
    }
}

It's clever. It's short. It's also hard to follow, and static analysis can't check it, because the method names are built at runtime. Rename getPhone() and nothing will tell you this code is broken.

The boring version:

if ($userDetails->getUsername()) {
    $user->setUsername($userDetails->getUsername());
}
if ($userDetails->getEmail()) {
    $user->setEmail($userDetails->getEmail());
}
// ...and so on for each field

Longer, repetitive, and completely obvious. Your IDE can navigate it, PHPStan can check it, and anyone can understand it at a glance. Boring beats clever.

A bit of homework: open some code you wrote more than six months ago. See how much of it you understand. It's humbling.

Can we remove comments?

if ($this->messageSender->sendMessage($message) === true) {
    // 3 means message sent
    $message->setStatus(3);
}

The comment only exists because the code isn't clear. Fix the code and the comment goes away:

$message->setStatus(Message::SENT);

(These days, Message::SENT would be a case on a MessageStatus enum. Same idea.)

Do we need comments?

Not all comments are bad. Some earn their keep:

/**
 * Populates template string containing placeholders with placeHolderValues.
 *
 * Inputs of
 *     $template: "Hello {name}. Prepare to play {game}"
 *     $values:   ["name" => "Jane", "game" => "monopoly"]
 * Returns: "Hello Jane. Prepare to play monopoly"
 *
 * Optional values are marked with a ?, e.g. {game?}
 *
 * @param array<string, string> $placeHolderValues
 * @throws MissingPlaceHolderValue
 */
function populateTemplate(string $template, array $placeHolderValues): string

A worked example, a note about syntax, and type information that the signature can't express. That's a comment that makes the code easier to use.

Are the comments up to date?

Every review, check that comments still match the code. A stale comment is worse than no comment, because people believe it.

Is the code obvious and explicit?

public function addAddress(string $address): void

What kind of address? Postal? Email? IP?

Renaming helps:

public function addEmailAddress(string $emailAddress): void

But nothing stops someone passing a postal address. A value object does:

public function addEmailAddress(EmailAddress $emailAddress): void

Now static analysis enforces it. (I've written about this in Your Types Are Correct. Your Code Is Still Wrong.)

Are we following project conventions?

interface LocationRepository
{
    public function findClosestTo(Point $point): ?Location;
    public function findByName(string $name): ?Location;
    public function findBySlug(string $slug): ?Location;
    public function searchForLocation(string $name, string $type): array;
    public function findAllByType(string $type): array;
}

Spot the odd one out. Every method starts with find, except one.

Conventions like this matter because they make a codebase predictable. When you've seen one repository, you know how they all work.

Write conventions down, along with the reasoning. On one project we used a #coding-standards Slack channel, with a thread for each decision so the discussion was kept alongside it. Expect a burst of these conversations when you first introduce code review. That's healthy. It means the team is working out what "good" looks like.

(And if a convention can be checked by a machine, write a rule for it. Custom PHPStan rules are perfect for this.)

Is the naming correct?

Agree one name for each concept in your domain, and use it everywhere: code, comments, tickets, conversation. A glossary in the repo helps enormously, especially in complex domains.

Use design pattern names correctly too. If it's called PersonFactory, it should be a factory, and it should create Person objects.

Has technical debt been documented?

Sometimes a hack is the right call. Getting a feature out before the Christmas sales might be worth far more than doing it properly a month later.

That's fine, as long as it's deliberate and visible:

// TODO https://trello.com/c/Aaa123
// Refactor to method
// ...some hacky code...

The ticket makes the debt visible to the business. The comment tells the reviewer (and future readers) that the hack was intentional, not a mistake.

Is the architecture good?

Is the code in the right place? Does it respect the boundaries in your system? Ideally, big architectural questions get settled before the code is written. But review is the last chance to catch them.

Are there any bugs?

Last on the list, but still on it.

Everyone should review

Code review isn't just for tech leads.

Juniors reviewing seniors' code is one of the fastest ways for them to learn how the codebase works and what good code looks like. And seniors learn more from juniors' questions than you might expect.

If someone isn't confident enough to approve a change on their own, build a culture where they can say "looks fine to me, but can someone else take a look too?"

Make your code easy to review

Reviewing is only half the job. How you prepare code for review makes a huge difference.

Talk first, review in the middle

For anything non-trivial, talk through the approach before writing code. It's much cheaper to change a plan than a finished pull request. For bigger pieces of work, get a review part way through, rather than only at the end.

Keep it small

Ask programmers to review 10 lines of code, they'll find 10 issues. Ask them to do 500 lines and they'll say it's good to go.

Everyone who's done code review knows this is true. Concentration falls off a cliff. SmartBear's guidance, based on a study at Cisco, is to review for no more than an hour at a time, and fewer than 400 lines of code at a time.

Tell a story with atomic commits

A big change doesn't have to be a big review. Break it into commits that each do one thing:

DEPRECATE: The price calculation service
ADD: Facade to 3rd party price calculation service
UPDATE: Use new price calculator code
REMOVE: Deprecated price calculation service
MERGE: Use new calculation service: https://trello.com/a/1234

The reviewer can follow the story one step at a time. Each commit is small enough to review properly.

Some things should always get their own commit: whitespace changes, class renames or moves, method or property renames, and Composer updates. Mixing a rename into a logic change turns a 20-line review into a 400-line one.

Bigger refactors should get their own pull request entirely.

git rebase -i is your friend for tidying commits up before you ask for a review.

Writing and receiving comments

Writing review comments

Don't be rude. This should go without saying. Sadly, it doesn't.

Criticise the code, not the person. "We" rather than "you". "We could extract this into a method" lands very differently from "You should have extracted this".

Give the problem and a solution. If a name is bad, suggest a better one. The author probably already suspected it wasn't great.

Link to further reading for bigger topics, rather than writing an essay in the comment.

Use "Let's chat". Some things are too big for a comment thread. Talk instead.

Use "Question". Sometimes the code is probably right, but you don't understand it. Prefixing a comment with "Question:" makes it clear you're asking to learn, not asking for a change.

Compliment good work. If someone's found a nasty bug or structured a change beautifully, say so. Even better, share it with the team: "Check out this PR, it's a great example of splitting work into focused commits."

Receiving review comments

Don't take offence. A comment isn't an accusation.

Do say if you disagree. The reviewer isn't automatically right. Talk it through. Usually both of you end up understanding the problem better.

Compliment good reviews too. A reviewer who spots a real problem has saved you from a much worse day later.

Keep on top of reviews

Code review is more important than new code. An unreviewed pull request is blocked work. Picking up a review should come before starting something new.

Review in focused blocks of 30 to 60 minutes, rather than trying to fit it between other tasks.

Be pragmatic. The default should be to review everything. But it's reasonable to agree that very small changes, and common, mechanical refactors, can skip it.

Make it part of the workflow

Here's the rule I'd put in place on any project:

Code can only be merged if CI passes and code review passes.

On GitHub, it takes a few minutes to enforce this with branch protection (or the newer rulesets) on your main branch:

• Require a pull request, with at least one approving review, before merging.
• Dismiss stale approvals when new commits are pushed.
• Require status checks (your CI) to pass before merging.
• Require branches to be up to date before merging.

GitLab and Bitbucket have equivalents. Years ago I set up Gerrit for code review, and it took a day and a half. There's no excuse now.

What still holds up

I said at the start that this was written before AI-assisted coding became normal. Reading it back, here's what I think has aged well.

Automate first matters more than ever. If code is being generated faster than people can read it, anything a machine can check must be checked by a machine.

Review the tests is still the most efficient way to find out whether code does what it should, whoever (or whatever) wrote it.

Keep it small hasn't changed at all. A 2,000 line AI-generated pull request is exactly the "500 lines, looks good to me" problem, only bigger.

Will I understand this in six months? is the question to ask about any code that nobody on the team actually typed.

What's changed is the rest of the picture: who writes the code, how much of it there is, and who (or what) reviews it. That's a topic for another article.

In the meantime, watch the full talk from PHP UK 2023.

What's the most useful code review comment you've ever received?