Building a code review process that works

A while back, I talked about the need for maintaining coding standards and how it simplifies the development process for the whole team. The next step is putting in place a code review process.

Why It’s Important?

Code reviews are very crucial for

• knowledge transfer
• avoiding small/common mistakes
• maintaining best practices throughout the dev team

Let’s take my current team for example. We are around eleven developers in the team, all producing code which needs to be reviewed. So yeah, that’s a whole lot of code!

Pushing code to production is easy. Anyone can do it, right? What concerns us is the quality of code pieces we are going to deploy. 

The code can be completely fine or it can be a piece which makes everything fall apart. To maintain high code quality, we all need to have peer code reviews. This does not mean that the team writes bad code that needs to be checked. We all are on the same team and we have a common goal, that is to deliver the highest quality product. And a code review process makes sure someone on the team catches the errors that somebody else might have missed.

You must be thinking, “Is it worth it”?  Absolutely yes. 

Not having a code review process integrated into projects can result in big problems. Toyota had to settle a $3 million lawsuit because they did not pay enough attention to code reviews. 

There were a lot of reasons why this incident happened and one of the reasons was an absence of a peer code review. After reviewing their source code they found possible bit flips, task deaths that would disable the fail-safes, memory corruption, single-point failures, inadequate protections against stack overflow and buffer overflow, single-fault containment regions, and more. The list of deficiencies in process and product was lengthy.

Obviously then, it makes business sense to make code reviews a critical part of your development process.

How Do We Do It?

General Development Process

Assigning responsibility

We have set a few guidelines around who will be responsible for the code review process:

• There will be one senior and one junior code reviewer for each ticket.
• Reviews will be done right after the daily standup, depending on how long takes
• When there are tickets added on the board, one person will review at least one ticket in a day
• It is both the code reviewer and developer’s responsibility to ensure that all tickets are in respective columns according to the latest update
• If there is any feedback from the senior code reviewer, it’s the junior code reviewer's responsibility to look what he/she has missed

Github Code review flow

Maintaining your code review process on GitHub is super easy. We can create new projects on our repo and use it as we want. 

Generally, we have four columns, which are:

• Ready for review: You can add cards (pull requests) to this column if your code is ready to be reviewed
• In review: Now it’s the code reviewer’s responsibility to move the card to "In review" column so that it gets updated on the branch and the concerned dev understands that his/her ticket is currently being reviewed
• Change Requested: Again it’s the code reviewer’s responsibility to move the card to this column if the review has failed standards. Then concerned dev will fix the issue and push the ticket back to “Ready for review”
• Closed/Done: If the card is in the  “Closed” column, that means the PR has passed the requisite coding standards

These are the aspects which we consider while reviewing code: 

• Best practices pertaining to Drupal standards
• JS specific coding standards
• SCSS specific coding standards
• Any piece of code that is adding technical debt. For instance: choosing an easy solution right now that might make it harder to implement changes later on
• Security issues
• Potential performance issues/threats to existing solutions
• Solutions being developed which are already available in the codebase and can be reused
• General standards like formatting, linting issues, comments, naming patterns etc. 

Making sure the process is followed

We regularly review our way of working against a documented process to identify if there are any gaps in our performance. We also look for ways to improve the process and ensure that it’s not a burden/blocker for anyone’s work.

The process is clearly defined and maintained on different platforms, depending on the client and teams involved. We usually use Confluence and highly recommend it to anyone who is reading this post.

Learning from our mistakes

We maintain a code feedback sheet where we mention common mistakes we need to avoid. Everyone on the team has access to it and can add points on where we can improve, new techniques to achieve certain functionality, coding patterns to avoid etc.

Goals of a Coding Review Process

With a finely tuned code review process in place, development teams can:

• Enhance the learning of individuals to become better programmers
• Improving the quality of codebase, even as it grows more complex as we scale
• Focus on not just quantity, but quality deliverable throughout
• Maintaining discipline within the team and understand the seriousness of spaghetti code

That’s about it! There would definitely be things I missed here or certain code review practices that are unique to your team. Leave a comment to let us know.