The Art of Code Reviews

In the years that I have worked I have worked with a few varied team setups. Small and big, depending on what you consider to be a big team. In one case I worked in a team of two where the other person on the team was the company's CTO. In all cases though, there was some form of code review that happened. Early on I saw code reviews as a lame and unnecessary part of a process. Then I saw them as a ritual to personally attack me and the code I wrote. But then I quickly saw the value and importance of code reviews, more so getting them than giving them.

To me, code reviews are more like an art than a science. There is no known algorithm for code reviews that works in all cases. Like art, there are some guidelines and a lot of room for expression. This means that getting them right is something that comes with experience. A lot of iterations and experimentation must happen before a working solution is found.

I will outline the guidelines that I think are essential for doing code reviews right. They will be in no particular order and in no way absolute.

The purpose

Before doing code reviews every team must set out a clear purpose of those code reviews. It does not have to be a single purpose, It can be multiple purposes. What is bad is doing code reviews just because it is an industry wide practice and a practice you think is good. Don't assume that everyone on the team will do the reviews with a similar purpose if there is no clear set purpose for them. This is important because it gives the reviewer a clear objective while doing a code review and the author an understanding of why they get certain feedback.

For example, a purpose of code reviews could be to ensure grammatically correct English on all public facing interfaces as well as proper internationalisation. The reviewer would be right to suggest an alternative English sentence and the author would not be left wondering why their English was dissected instead of the awesome code they just produced.

The reviewer's etiquette

As well as setting a purpose, teams should be clear on how feedback is communicated during these reviews. A clear tone of review must be set. When humans are involved, even in places where they should not be, feelings and egos are involved. To some people, a question of why a certain construct was used over another may sound confrontational. Maybe a suggestion of an alternative construct may sound less confrontational and a setting for a discussion.

Code reviews should not be used to ridicule another team member for not choosing the best algorithm. Code reviews should not be used to impose personal preferences on other team members. They also shouldn't be used to make fun of other's mistakes.

The author's etiquette

Teams should also set out clearly what is expected of the author after a review. This ties to the point above. Civility during reviews is a must to avoid bruised egos and hurt feelings. Teams should be clear about whether the feedback is to be considered as advice or a request for change. The author should address every concern raised by reviewers. The author should be able to open a discussion of a solution they have implemented.

A suggestion for an alternative way to do something does not mean you are wrong. The author should not get defensive. In fact, to me, this is a sign of inexperience. An experienced author is always welcoming to critique and willing to listen to reason. "I like it this way" is not a suitable response to "how about doing this a different way". Asking why and being able to discuss the different solutions is a more acceptable response.

The platform

Teams should pick a platform for reviews and stick to it. Even if some parts of the code review process take place off the platform, an effort must be made to capture those parts on the platform. Ideally the platform should be part of the process of getting code into production systems. Luckily, tools like GitHub PRs are in abundance and they are perfect for something like this.

Taking conversations offline and discussing solutions on a whiteboard is good. It is an opportunity to teach each other different ways of solving problems. However, bringing those resolutions back to the review platform is a must. Anyone who was not present in the offline discussion should not wonder why there are unanswered queries on a PR and yet it was merged.

Echo

Using the guidelines above, teams should experiment to find what works for them. I have seen cases where organisations make code reviews required without setting a clear purpose for them. This causes confusion and endless arguments.

Reviewers should colour within the lines. Authors should accept constructive criticism and not take feedback as a personal attack. No one should be defensive during the review process because it is intended to be about the quality of the code and not the person.

As with every art, only practice brings improvement. No amount of theory can match that. Code reviews should be culture within an organisation. They should be allowed to evolve and match the times. They can be a great way to introduce new team members to the code base. They can also be used by team members to keep in touch with changes on a code base.

If there is something I have missed or something you disagree with, you can hit me up on  twitter for a discussion.