My personal guide to code review

Just like any other process, code review is easier when you know exactly what are the steps needed to successfully finish the task.

I have been developing my personal process to code review for some years, but I never wrote it down before. This post is an attempt to formalize the process. My idea here is to start from the very basic, and grow this guide as I'm able to detect more patterns.

I encourage anyone who's reading this guide to write your own guide, and share with us. It'll help both you and everyone else reading your method.

Categorizing

Usually, I'm able to categorize the PR in one of these categories:

• Bug Fixes
• New features

When the PR is not clearly one, and preferably, only one of these things, we will have the first red flag. The team may need to discuss the context that this PR was created to make the process of reviewing easier.

Bug Fixes

1. Understand when and what conditions the bug happens.
2. Reproduce the bug in the "master" branch so you can see the bug in the front of your eyes.
3. Checkout the branch that's fixing the bug and try to reproduce it again.
4. Run the regression tests. The expected is to pass.
5. Rollback the changes that are fixing the bug, run again the regression tests. The expected is to fail.
6. Review code line by line — I'll go deeper in this topic in some future post.

New Features

1. More usually than not, the Pull Request of a new feature is just an independent deployable part of a bigger delivery. In this case, if you are in a team that is good at breaking a large task in small ones, is expected that you’ll have the instructions of what needs to be done in that PR to allow the other parts that are coming. Focus on that. If, for some reason, you don’t have information enough to get the idea of that PR, do not think twice about asking as many questions as needed.
2. As we are adding new code, it’s important to check if this new code is following the patterns of the team.
3. The tests should not test only the “happy path” when everything works. It also should test what must happen when somethings goes wrong.
4. If you are working on an API, you probably have some documentation for this API. In this case, you must check if the changes made in that PR are reflected in the documentation.
5. As we do in the bug fix, we should go line by line here too.

We might also consider the possibility of a third category. Sometimes we are just refactoring some code, updating a dependency or something like that. In this case, we could say that this is an "internal change". I won't describe those yet because they vary a lot and I couldn't figure out the categories related to that kind of PR.

In the next post, I'll write about what are we looking for when doing a line by line review.