_ Blog / Technology, Management

Code Review: Create The Culture, Learn The Best Practices

by Gábor Zöld
/ July 17, 2019

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

Code Review Best Practices

Code Review Checklist

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

Source: smartbear.com

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.

Source: smartbear.com

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.

What you should have on a code review checklist

The most essential questions one should ask when doing code review are the ones checklists should be built around. The highest priorities generally are:

Can I understand it easily?
Does it stick to our coding guidelines?
Are there duplications in the code?
Can I unit test the code easily?
Is there too much going on in one function/class?

Code review checklist example

Source: www.evoketechnologies.com

Short Code Review Case Study

Gloria Metrick

Owner

GeoMetrick Enterprises

When I was a young programmer, I was fortunate to be part of a huge project where we had 45 programmers automating an entire manufacturing building. That project was much helped by the strict code review and process rules. In the years that followed, I was able to create code review standards for the variety of projects I worked for and on. The systems I currently work with are so complex and have so much code that we often do code reviews. However, when we do them, they tend not to be particularly strict.

Code Review Rules for the Most Likely Success

Code review standards must be created and maintained by the technical team.
They must be supported by the management team.
Follow the standards strictly. If they don't work, send the issue to the code review committee to resolve. Don't try to skip over them.
Standards are living documents. If they have an error or if they need a change, then review them, discuss them and change them.
There is no place for feelings in code review. No one should be making it personal. In that way, it's easier to justify to others why it isn't personal. If any reviewer starts a sentence with, "I like" or "I don't like," it's a sign they're not understanding the process.
Code review should have deadlines on it. Everyone should be given enough time to prepare for and execute the code review in a fairly short timeframe. In addition, the programmer should be given some time window for changes, so the team can re-review the code to pass it to the next step.

What the Code Review Process Looks Like

Coding standards must be created BEFORE you write any code. If you wait and make people change their code, they will dig their heels in so hard that you will have an issue.
Once standards are created, people will write their code.
When the code is as good as the developer can get it, where all the standards were met, it compiles, and possibly some initial testing has been done (it's fine to do a little testing before formal unit testing, just to ensure it doesn't blow up), then the code review team can be selected.
No programmer should review their own code.
If it's a large project, have several people review the code. If it's a small project, just one or two. Don't keep making the same people review code unless there's truly no one else. They get burned-out.
Iteration happens. Code is seldom 100% perfect. Every programmer should expect changes.
Programmers are allowed to defend decisions they make or explain themselves, but in the end, the review team should make the decision. If there's a question to send back to the code review team, then it is possible there's some doubt about what to do.
Follow up with the programmer to verify they made all the changes, as required.
Iterate until the code meets the standards, then go to the next step.
If code fails the next steps, the same reviewers who initially reviewed it should review the changes to ensure all changes still meet the standards.
Don't create standards more complex than you can enforce. If you have a tiny team, your standards should be more basic than that of a huge team with a lot of people. Otherwise, you'll drown in the paperwork, basically. Large teams need more structure, as well, and the additional rigidity of the code review standards will help them manage the process. This is a common reason for the failure to enforce standards, along with the lack of team buy-in, or inactive management support.

Keeping Code Review Constructive

All programmers might not technically be part of the code review team, but they should be allowed to request or suggest changes.
All programmers must be trained in the code review standards.
Code review and other standards aren't a one-off meeting. In ongoing meetings, all programmers should be made aware that code reviews will take place, are not personal, and are going to be taken seriously.
This is where phrases such as "I don't like the way you..." or "You're personally picking on me..." should send up a red flag. If a reviewer or reviewee acts as if there's something personal going on, they need to be corrected nicely but firmly. It seldom has to be escalated to the project manager or group manager, but it's an option in extreme cases.

Code Review Tools

This varies by platform. Different groups of people tend to like different tools.
With that said, pick a code review tool. The code review committee should pick the tools. Everyone MUST use the same set of tools for the step it's intended for.
For example, in my industry, tools such as Diffchecker are popular among those languages that don't have other types of tools associated with them. Visual Studio is also popular, since some of our industry uses platforms where this tool is used. Basically, whether you use a straight text difference checker or something more sophisticated, you do want to easily show changes from what was shown in the review to what the programmer changed for the next go-round.
To track everything, some people have database programs; others track in spreadsheets. There are all types of ways to manage this. The bottom line is, as long as you have a clear way to track the versions and changes versus what is required from code review (which could be written up by marking up the code as a separate document), you can track it all back.

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

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.