Code reviews are awesome, when done properly

Code reviews are awesome. Seriously, if you are not doing code reviews you should start doing them right away. There are many good reasons for doing so.

Code reviews are a learning opportunity

When done properly, code reviews are awesome learning opportunities for junior developers. There is no better way of growing as a developer, than having someone well seasoned in front of you, explaining to you, line by line, how to make your code better.

But wait! Code reviews are awesome learning opportunities for senior developers as well. You know that saying “an old dog does not learn new tricks”, don’t you? Well, there is nothing better for a senior developer than a newcomer to the craft asking all kinds of questions about a codebase. If makes you reconsider each and everyone of your decisions, and most times, it makes you realise how many things you do that you should probably not be doing. And how you do those things without any particular reason.

Code reviews help improve the code quality

This should be obvious, but apparently it is not. The main goal of a code review is being sure that the code quality is up to the team’s standards. Now, what are those standards might be a theme for another post (or a thousand posts), but the key concept here is that opening up the code for review, everybody gets to read it, discuss it, and improve it through suggestions and comments. Which leads to…

Code reviews enforce collective ownership of the code

You have probably heard it a thousand times: code should be owned by the whole team. There is no better way to make the team feel like the code is owned by everyone than making everyone actually own the code. And that happens when everyone’s suggestions and comments are incorporated into the code base. More importantly, opening up the code for review avoids what I call dark zones, chunks of code that none has actually seen, and that no one knows what they actually do or how they actually do it. Again, collective ownership FTW!

Code reviews help share knowledge about the code base

Here comes Captain Obvious! Yes, if everybody is supposed to read everybody else’s code, understand it, and being able to comment about it and suggests improvements to it, as a side effect, that means that everybody understand the whole code base. And that is awesome!

Code reviews avoid nasty surprises.

We have all been there. James did work on the new feature, and his code was complete. Until we tried to integrate Jame’s feature into the new release. That’s when we found that, apparently, James has a different definition of “code complete”. Code reviews will rise to the surface issues with the code as early as possible. If the feature is not implemented as it is supposed to, a good code review will catch that.

Code reviews motivate developers to write their best code

If you know that everyone else is going to read your code, you are going to find that extra pinch of motivation to avoid duplication, or to come up with better names for your methods and variables, or to break down that very long method into smaller methods.

Code reviews create a team’s culture

Yeah, the company culture. That thing every company on the face of earth wants to create, that thing that is supposed to act as a mesh, as a framework, that supports all the engineers, that binds them together, that makes easier for new hires to catch up and blend into their teams. A healthy system of code reviews makes wonders when it comes to create that. There is no better mentoring for a new developer that his first pull request open for review. There is no better way to make that new hire feel as he is growing as a developer than receiving continuous feedback about his code and being able to provide feedback to his colleagues about their code.

All that is cool, and everything, but how can you make sure that everybody understand code reviews as all of the above?

Well, I am very sceptic about this whole Scrum and Agile Cult. But there is something in that process that I really like: the idea that teams should be self-organised. Give a team the opportunity to figure out how and why they want to do their code reviews, and they will come up with something good. Good, I say, because it will be something they all will stand by, and thy will all abide by.

That is what my team did, And this is what we came out with (edited to remove some company-specific items)

Code reviews: rules and regulations

Rule #1

Code reviews are a learning opportunity, both for the person opening up his code for review, and the persons doing the actual review, and they should be approached as such.

Rule #2

Code reviews are done for one reason: deliver the best possible code.

Rule #3

Comments in the code reviews shall be performed with rules #1 and #2 in mind.
Corollary 1: Comments in the code reviews should be backed up by arguments and facts, not by personal preferences.
Corollary 2: Refrain from adding comments that provide no value. If a single comment can achieve the same result of 10 comments, it is better to make that one comment vs. 10 different comments in 10 different places.

Rule #4

The result of a good review should provide:
– A list of things to update. There should be a clear definition of what to update.
– A list of things that require further discussion.

Rule #5

In order of importance (descending) a review should focus on:
– Global intent of the code (e.g. does it do what it is supposed to be doing?)
– Architecture (e.g., does it follow the technical design?)
– Gaps in the unit tests (e.g., does it cover all the requirements?)
– Code readability (e.g., does it conform to code clean guidelines?)

Corollary: While code style is also an integral part of code readability, it is not considered an important aspect of the code review unless it blatantly violates the agreed upon standard.

Leave a Reply

Your email address will not be published.