Code Review: Create The Culture, Learn The Best Practices

Code review is one extremely powerful tool in a software developer team’s arsenal, yet it’s often neglected or used in a way that leads to toxic behavior, causing more harm than good. Code reviews done right can help your team share knowledge, which helps with mentoring interns and junior developers tremendously, but an experienced developer can always improve as well.

Reviewers become familiar with parts of the code they otherwise would not get into, so it spreads product knowledge within the team. It guarantees there is a person to go to, even when the author of a specific part of the code base is not present. On top of that, developers will be much more attentive to code quality standards if they know others will review their work when it gets to a pull request.

All this makes the development process smoother, which is incredibly valuable, and here, we will show you how to make use of code reviews while avoiding the possible drawbacks of code reviews gone wrong.

• Create the Right Code Review Culture
• Tone is key, stick to the facts and explain
• Phrase suggestions as questions
• Avoid sarcasm and emotions
• Limit and separate nitpicking
• Address all comments
• Prevent toxic behavior
• Code Review Best Practices
• Don’t rush it; limit review sessions
• Set up metrics and goals for code review
• Use automated tools
• Annotate code before review
• Take it to a personal discussion
• Include everyone
• Code Review Checklist
• What to focus on with a code review checklist
• What you should have on a code review checklist
• Code review checklist example
• Short Code Review Case Study
• Conclusion

If you want to read more real-life code review case studies then download our free guide here below.

Create the Right Code Review Culture

Besides code review taking time and effort, the main reason for some teams being reluctant to apply is bad experience caused by toxic behavior during code reviews. In a worst-case scenario it can really hurt a team, so let’s take a look at how to create a culture where everyone is comfortable with receiving a code review when they send a merge request.

The importance of writing can't be overstated; it's all about writing good comments.

👉 Learn the ins and outs of Monzo's renowned engineering culture!

Tone is key, stick to the facts and explain

When it comes to code reviewing, nearly everything depends on the tone and the way the feedback is phrased, so you should make sure it’s done the right way.

Don’t phrase opinions as facts. For instance, even when it’s a valid point, avoid stating something like this:

“X component should be stateless.”

It would be better to stick to the facts and explain further, like this:

“For A reason, X component could be stateless, which will improve B. Check *this* documentation.”

This is a more productive way of discussion, and it could prevent lots of issues when it comes to style preferences, which is usually the area where opinions come up. It is especially important to watch out for when the code reviewer is a senior developer, as their position of authority could prevent a healthy discussion on the right way of wording review comments.

Phrase suggestions as questions

Putting suggestions as questions can help to start and to drive forward a conversation. It’s important to separate them on tone, as not all questions are made equal when it comes to code review.

“Why didn’t you do X?”

This question heavily implies the person really should have done X, which not only makes them feel bad about it, it may not even be a correct suggestion, possibly leading to a fight. Ask the same thing instead like this:

“What do you think about doing X here? In this case, it would do A better.”

This is a better choice, as it doesn’t make anyone feel inferior whether it’s right or wrong, and it opens a dialogue, leading to the best solution at hand.

Avoid sarcasm and emotions

It is never a good thing to leave a comment like:

“Did you even test this code before sending it?”

The problem here is not only the tone, but it also does not address the specific issue at hand. Using emojis is counterproductive as well, since keeping emotions out of code review on both sides is essential. Specifying the requested changes is a must, while making yourself look superior or mocking your colleague is a hard no. Phrasing it like this is far more productive:

“The code breaks when X. Could you please address this?”

As a rule of thumb for a good code review: it’s better to give recommendations with reasoning than it is to just point blank tell others what they should have done in the first place.

Limit and separate nitpicking

Nitpicking on things like structuring tests in a certain way or putting brackets in the same or the next line often isn’t helpful, and it could take away attention from more important notes. So, it should be marked as a low-priority comment, or even dropped completely unless it’s about an agreed-upon coding convention of the team.

It also tends to lead to a massive amount of notes on the same thing. It’s usually more helpful to just post one comment, like:

“I found you tend to do X. Our style guides say we shouldn’t do it. Could you take a look at this, please?”

When notes like this get common, the trick to a good code review usually isn’t to mark everything, as people aren’t effective in catching every single instance of these stylistic inconsistencies.

A team should implement guidelines for coding style, to make sure all developers agree on one way of doing things so they can watch for it themselves. Also, using automated tools to root out minor inconsistencies, and to keep the code up to standard, is extremely helpful in saving time and effort for reviewers.

Address every review comment

When you’re on the receiving end of a code review, you should be mindful of some things as well. One key element is to address all the comments left for your code. If there is an argument to be made to keep something in, it’s useful to give at least a reply to the comment, and to explain why the solution should stay.

This should make sure nothing important is missed, and also that there is no bad blood within the team. Simply ignoring suggestions will surely leave a bad taste in the mouth of the reviewers, wondering why they even bothered, and regularly repeating it is detrimental to the entire team's code review culture.

Prevent toxic behavior

The most important thing to state here is never to partake in toxic behavior. Even if it seems to be going around in a team, it’s best to refrain from it, as just one person can make a big difference as a first step.

The other thing is not to tolerate it from others, even from high performers. Make sure everyone is clear on the rules and expectations, as one person in a team ignoring all the guidelines above regularly could be enough to corrupt a team’s work environment.

If it ever happens, it’s best to address it head-on as soon as possible, before it leads to more serious problems. For instance, work slowing down, people avoiding peer code reviews, or in extreme cases, people leaving the company.

Code Review Best Practices

Once you put the right culture in place, let’s see some additional practical things to approach code reviews with, and make them as effective as possible.

Don’t rush it; limit review sessions

The first thing to nail down is that code reviews should not be rushed, or else it loses its point, as no mistakes can be caught when people just want to get in and get out. Developers can usually get through effectively about 300-500 lines of code in an hour. Statistics show after the first 60 minutes, effectiveness drops significantly, and it drops even further after 90 minutes.

Limiting review sessions to about one hour and/or 300-500 lines of code is a good policy to use.

Set up metrics and goals for code review

Setting up a process is great, but setting up metrics to measure its success is just as important. Using external and internal metrics can both be useful in deciding if the code review process needs revising. It’s important to remember, it should be a modifiable process along the lines of team, product and company needs.

For example, external metrics could be support codes, defects found before release or defects reported by users. Internal metrics for code review may be inspection rate or defect rate. The results should not be used to blame people, but to assess the process and to make sure it is as effective as possible.

Use automated tools

ESLint or TSLint are the automated tools we use at Coding Sans. They are great for enforcing stylistic conformities. They spare the developers from having to put in a lot of extra effort into following the outline perfectly on every line of code and trying to catch all of them during code reviews.

It’s a great rule of thumb to get all the work possible done automated on formatting and style, so when it comes down to peer code review, everything will go smoother and quicker without wasting precious developer time.

Annotate code before review

It’s incredibly helpful to properly annotate a piece of code before sending it to be reviewed. Making self-describing code is very helpful, but at complex parts of the code, the right annotation can make the review process far more efficient.

The reviewer needs to spend less time figuring out what the code does. Proper initial explanations may also resolve some possible issues before they even come up, or shorten the discussion on them, leading to a quick and efficient conclusion.

Take it to a personal discussion

When a back-and-forth emerges in a code review, leaving notes becomes an inefficient way of addressing the issue. It’s important for the developers to recognize when it’s time to take it to a personal discussion.

As a rule of thumb, it often may be useful to switch to a personal discussion as soon as a reviewer gets a response on one of their comments they don’t want to just leave in place, as it often leads to a longer and more nuanced conversation. If it comes to a decision stalemate, a more senior third team member should break the tie.

Include everyone

It is important to involve every developer in the code review process. Junior developers learn a lot from them, it spreads product knowledge within the company and team, and seeing reviews from both sides helps everyone become comfortable with the process itself.

Sometimes it is more useful to have a senior developer review certain key pieces of the code, but a peer review is incredibly helpful as well, so it's best to include everyone.

👉 Further reading: Code review in the age of Github

Code Review Checklist

Using a code review checklist is an essential tool to keep it effective, even for senior developers. Let’s see the baseline on how it should be done.

What to focus on with a code review checklist

If you are not using a code review checklist yet, going straight to a very nuanced and complicated wish list is usually ineffective. Instead, consider where your company and team should improve, and what the most important things are to watch for in the product. Make sure it’s followed, and keep building from there using feedback from everyone involved.

Never forget: code review checklists should always be living documents, and they should be revised from time to time as necessary. Another useful tip is that many teams use different checklists for different programming languages, reminding the reviewers what to look out for.

Conclusion

To summarize it all: the most important prerequisite for effective code reviews is to have the right culture in place. People discussing and understanding each other’s position is far more effective in practice than people telling others they’re wrong.

Once you have it in place, take the next steps by setting up a code review checklist and by implementing the industry’s code review best practices. Never forget: you need to tailor everything for your team, product and company, so don’t be afraid to make changes, and always keep an open mind.

Contributors:

Steven McCord
CTO
Mobile Posse

Jaanus Rõõmus
CTO
Messente Communications

Brujo Benavides
Erlang Developer & Trainer
AdRoll

Gary A. Galehouse
VP of Product Development
Keyfactor

Lauren Hasson
Founder
DevelopHer

Rob Fraser
Director
Pragmatic Digital

Ian Wang
Front-End Lead Developer
Codementor

Mauricio Giordano
CTO
InEvent

Soumik Sarkar
CTO
Crosscode

Marc Fletcher
CTO
Echobox

Hernán Rivas Acosta
Software Engineer
Erlang Solutions

Photo by Rick Mason, Unsplash