The code review – our process

Yet another article about code review and in our last article, my colleague Eldar has taken a chunk of code and pointed out all the mistakes and all the questions one simple piece of code can generate. Basically what he did was a code review. 

Code review
2019-06-19 Karlskrona Softhouse. Foto: Jonas Ljungdahl

Now we are going to present how our code review process looks like.

The value of code review

Code review is an essential part of development (as is writing unit tests by the way), so actual code writing is only a small part of the complete development cycle. First question is:  What is the value of code review? 

As you may assume we have the usual suspects:

Prerequisites

For code review to run in an efficient flow, establishing some sort of Continuous Integration/Continuous Delivery system is an absolute must have. My personal recommendation is the Atlassian toolset, but whatever works for you.

Once Pull/Merge Request is created, the automatic builds are triggered. Builds include code-style check, code standard check, spell-checker, unit-tests check, functional-tests check. Even if you don’t have all of these checks setup, I would recommend their setting up even with minimum functionality. Then with time and resources they can be expanded to bring greater value. This way you are reducing things to look at as a reviewer, and you can skip silly indentation comments and things like that. Only thing you must focus on is functionality and if it’s possible even architectural decisions. Our ideal scenario is two reviewers, where one of them must have a deeper architectural knowledge on code under the review so he/she can understand the deeper implications caused by merging of the reviewed code.

Checklist

The Code review process is a flat hierarchy and no developer is spared from code review. Even if everything is perfect (very, very, very rare case), there is a learning opportunity for the reviewer. Obviously, not every check is applicable to every project, but here are some things we pay attention to, and I believe you can use some if not most of them.

Preparation

Purpose

Implementation

Dependencies

Maintainability

PR Comments

Since Pull Requests are a form of communication between two or more persons, some rules and guidelines need to be established, in order for the process to be as fast as possible. Here are some guidelines for both reviewer and code owner.

Reviewer:

Code owner:

Conclusion 

Once you are a code reviewer, code becomes your responsibility as well, so everything that gets merged that can cause problems on the main branch is shared by fault. Code review is not a 15 minutes task. It takes time and that time should be planned accordingly. Just as with coding, my suggestion is to take breaks, do not review for more than 60 minutes at a time.

Missed the first part of the blog series? Read Part 1 in our blog series about coding.