How to Beat the Game of Code Review

Reviewing code is an awesome activity.

Whilst writing new code, your life is on the line. Every line you write may contain a bug. And that bug comes after you when you least expect it.

Reviewing code is… quite the opposite. There is a codebase that may or may not already contain bugs and issues. Your task is to find them. If you do your job well, the code only gets better from here.

Code review is a game, and depending on how deep we go, we might be rewarded with the ultimate payoff: placing a valid review comment.

Let’s look at the different stages of code review, and learn how to beat the game!

Level 1: The Glance

LGTM!

In far too many places, code review happens like this: Reviewers usually don’t feel much of a responsibility for the code that they look at. So all the code gets is a quick glance, after which the code is approved with a comment stating “LGTM” (looks good to me).

Especially large PRs suffer this fate:

In the game of code review, having a quick look at the code allows to spot some obvious issues, and sometimes catch a few code-style issues. But in my experience, those are rarely the big issues in code changes.

Anyway, if you made it past the glance without clicking Approve and typing LGTM, you’re ready to move to Level 2 and actually looking at the code :)

Level 2: The Human Compiler

While glancing over source code, we might spot some obvious issues, but our mind does not actually understand how the machine interprets the written code.

To really understand the control flow, we have to mentally execute the code, and consider different kinds of input and edge cases.

By mentally executing the code we can find tricky bugs. If the code is hard to mentally compile, there’s probably an issue with the technical design of the code.

From my own experience, I notice that this step may take quite some time. It requires you to understand everything that is happening, which is often opaque. But this is also the step where I usually post most of the comments, so it’s very effective as well :)

Finishing Level 2 is enough for relatively simple or non-critical changes. If we’re dealing with a more critical or extensive change, it’s time to move to Level 3.

Level 3: The Checkout

It’s time. We’re going deep.

git checkout -b feat/feature-under-review

For Level 3, we’re actually compiling/running the code, and manually testing that the feature works.

This may cost a lot of effort, since some features require setting up some data or application state. But it’s the most effective at verifying the solution.

You might even submit a fix for an issue you find ;)

Usually, for most reviews, after compiling and running the code, we should be quite certain about its correctness. But if we feel we have not yet covered all, it’s time to move to the Boss Stage.

Boss Stage: The Ultimate Test

When you opened the Pull Request, you never hoped to make it this far…

Yet here you are.

Three hours of your day gone on what should’ve been a simple code review…

Now that we’re here, we better make it count.

Reaching the Boss Stage usually means that you encountered some challenges with the code under review. Just a couple comments won’t cut it. The code needs to undergo proper testing.

Modifying the code under review is a sin. Adding a couple tests, even if they fail (perhaps they should even fail), is not.

Now is the time to revisit the task’s specification and create tests that align with it. It’s the ultimate test to determine whether the code under review is correct.

Once we make it through this Boss Stage, it’s finally time to ping the creator of the Pull Request and have them consider your comments.

And then, of course, you need to re-review! Because, well, this code is now your baby too :)

Being a Completionist

Do you know those crazy speed runners that know a game through and through, to the extent where they can beat a game that would ordinarily take several hours in just a couple minutes? How did they get that good?

Well, by pouring thousands of hours into the game. That’s why.

With code review, it’s not necessary finish all levels. In general, at least Level 1 and 2 should be completed. Level 3 is for very important changes. The boss stage is reserved for only the most critical of changes.

I’ve sometimes poured hours into a code review, and did not find a single major issue. It felt strange, because lots of time was spent, and in the end I had nothing to show for but the same green checkmark that I could’ve given with a single mouse click.

But other times, I’ve caught major issues before the code hit production, just because I took the time to compile, run, and understand the code.

When reviewing code, you should constantly reconsider the confidence you have in the correctness of the code. Is the confidence high? Then you might just approve it. Is it low? Then be sure to run the code, and perhaps even add a few tests to it.

This article was absolutely written whilst procrastinating reviewing a big pull request. It took some mental preparation, but I’m finally ready.

I’m going down to the Boss Stage 😎