Participants in Code Review

Posted by

You just finished putting the final touches on the feature you’ve been working on for the last day, and you’re ready to fire off a code review with your colleagues. The next question you may ask yourself is who to include on that list of reviewers. The first instinct is to go with people you’re familiar with, who may be less critical of your code. Unfortunately, that’s not in the code’s best interest, the organization that owns the code, or your own. A code review needs to find defects to bring value. You won’t do that if you don’t involve the best defect finders in your team.

We’re going to answer two questions in this article. We’ll have a look to see how many people should be involved in a given review and then we’ll have an in-depth look at all the possible combinations of developers that could get involved in your review.

How many people to involve?

Before jumping into the participants, let’s start by having a look how many developers it takes to spoil the “code broth”. It was surprisingly easy to find a consensus (across multiple references and online discussions) saying that anything beyond two participants in a review will provide little to no additional gains.

For starters, the person who prepares their code for review will often put in a better effort to write high quality code, knowing that someone else will be reviewing and judging it. The first reviewer to get to it will generally find most of the high-impact issues that need to be resolved, with a second reviewer finding the issues that the first missed.

While more reviewers might find more issues, they’ll most likely have significantly smaller impacts and not be worth the additional cost that you’ll incur for having additional reviewers and defects to correct. It could also have a negative impact on the review if the number of comments becomes un-manageable. That being said, the review might be finished quicker if you invite more than two reviewers and close the review once at least two reviews are completed.

Be wary of complacency and indifference as you increase the number of reviewers. Every reviewer thinks that the other developers on the review will find the defects within the code, putting less pressure on themselves to do a thorough job.

Now that that’s settled, let’s have a look at the most common (and not so common) options for reviewers.

The Players

Technical lead

The tech lead is responsible for ensuring best practices are followed, architectural guidelines are respected, and be an expert in the programming language of choice in your team. For all these reasons, the tech lead will be able to provide some of the best feedback for your review, making him or her a no-brainer to invite to your review. The tech lead often has many responsibilities and may not have time to be the first reviewer. However, they’ll still most likely be able to provide you with some higher level feedback that other developers won’t have access to, such as future development plans that might affect how you solved a problem. Review Value: High

Senior and Intermediate Programmers

This is the sweet-spot when it comes to reviewers. They generally have the time needed to dedicate to your review, and are more than willing to share their knowledge of a development platform. Often times these guys and gals will be experts in a single language, knowing the slightest nuances and minutiae that most people aren’t aware of. They also know when to mention a defect and when not to. Review Value: High

Junior developer

While it is true that everyone will bring a different perspective to the review, the lack of expertise in the language and business context will reduce the value of their review. There’s an argument to be made that even if a junior can’t contribute to the review they will still learn from reading other people’s code. On the other hand, it’s debatable whether a code review is a good place to learn the best practices of your team. Review Value: Low

Developers who don’t speak your coding language

There’s a particular case where this could be useful. A back-end developer reviewing front-end code (and vice-versa) could notice an issue or potential improvement in the interfaces between the two parts of the application. It will be difficult for this reviewer to provide meaningful comments outside of this scenario. Review Value: Low

Tester

Inviting a tester to a review should be to accomplish a different goal than finding code improvements. The tester may see obvious bugs and defects that can be caught before the code even gets to the QA stage. The tester will also gain some insights to the inner workings of the application, providing ideas on how to test the limits of the application. That being said, the tester won’t help to find the technical defects (say, a promise that won’t return as expected) that could be present in the code base. Review Value: Medium-Low

Architects

Similar to tech leads, the architect will bring an understanding of the application and system as a whole to the discussion. This is especially useful early on in the life of a project, when still modeling the domain and wider architectural decisions. Review Value: Medium

Person with very different view of coding than your own

It can sometimes be confronting when facing a review with a developer who looks at code very differently from you. Suggestions of massive changes that might take hours (or days) to implement could lead to frustration and lack of engagement. It’s important to have a technical lead or one of your best developers on the review as well to help decide if the alternative approach should be taken. Review Value: Medium-Low

How to choose?

Often times, we’ll end up choosing a few members of our team without giving it much consideration. It’s to your advantage, however, to consider the above reviewers and select the ones that will provide the most bang for the buck on the code you just wrote. The ideal mix will get you someone with a detailed understanding of the language in question, as well as someone with more global knowledge of the project.

There will be times when a key member you’d like to have participate doesn’t have time to complete a review. In these cases, you’ll have no choice but to leverage the people who are available. You should still try to find a way to get that key person involved, even if it is in a lesser capacity as a second or third reviewer doing a sanity check.

It’s also possible the organization you are a part of will have review guidelines that dictate who must be invited to a review. Whether you’re the one deciding who does reviews or just one of many developers using the guidelines, always make sure that the process continues to make sense for your scenario and raise a flag if it doesn’t.

Yet another possibility is to have a reviewer by feature or section of code. You can’t be an expert on all the code in your application and champions for a given part of the system, such as authentication or authorization, can be a way to ensure that each section of code gets the attention it needs.

In closing, there’s no one rule for choosing among different types of reviewers. The one rule you do want to stick to is choose the reviewers who will give you and the code the most value. And no matter what you decide, stick it with it!

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s