An open approach to code reviewsJune 18, 2013 | 10:58 am
One of the key aspects of every software product is the quality of the product itself. This is often measured by taking the number of bugs, performance metrics, the amount of memory needed to run it, and shown to the world in the form of attractive reports and graphics. However, there is one aspect that is very difficult to measure but extremely important: code quality. How can this be measured? Measuring code quality can be a very subjective process, so instead of relying on reports and tools to generate cool graphics, the smart approach is to integrate the code quality assurance process into the development cycle. Today I’ll explain how adding a code review process to our development cycle has helped us deliver better software.
It’s all about culture
There are many products that are great at controlling the code review workflow, such as Gerrit or Review Board. They provide a convenient way to review the code, integrate perfectly with CI systems, and enable you to control what can be merged into the source code tree and what should be rejected. This kind of tool works well for big, distributed, or heterogeneous teams, but may introduce too much overhead for small ones. When getting things done is a must, all the bureaucracy generated by these tools may not be worth the benefits they bring.
This was the case at Abiquo. We don’t have a big engineering team, and we all knew we needed some way to improve the quality of the code we committed. We all understood the need for code reviews and the benefits of having other people validate the code before it is accepted, so we opted for a more relaxed approach: using the Github pull request system to commit every single change.
Every pull request has to be validated by at least two members of the engineering team and cannot be merged until it has their okay. Once it has been accepted, the creator of the pull request can merge it.
One could ask what would happen if the creator merged the pull request without waiting for the validation, but the answer is easy: it does not happen. It is all about culture. We all understand the benefits and the need for code reviews and we imposed the process on ourselves, so naturally we respect it.
And who reviews the code? Anyone! All engineers have the power to approve or reject a pull request by expressing their opinion in a comment (but only the creator is allowed to actually merge or close it). There is no formal process that sets the reviewers, and no tool integrated with the CI system that pushes the changes once approved. We just trust each other, and it works.
This kind of altruistic review process works fine if you only have a few repositories to watch, but we have more than 30 different repositories where we commit regularly, so we have built a code review dashboard that helps give us a global vision of what is going on. It shows which pull requests are pending review, which have been approved and can be merged, which are old and should be reviewed ASAP, which have no reviews yet, and so on.
The dashboard is configurable and deployable to Heroku, so you can just download and try it with your Github account.
In just a couple of months since we adopted the code review process, we have seen many benefits:
- There are considerably fewer issues in our internal releases.
- We are all improving our coding skills and learning a lot from other points of view.
- We have a more global vision of the changes in the platform.
- This process indirectly helps the QA team to test without being blocked by minor issues.
- We write better software!
Being a small team, sharing the same culture of software development, and having this open approach to code reviews has allowed us to introduce the process with practically no loss of agility and helped us deliver better software since the first day of its adoption.