Analysis / Contributed / Top Stories

LinkedIn’s Tips for Highly Effective Code Review

27 Sep 2017 2:00am, by

 LinkedIn recently passed the milestone of having conducted one million code reviews. The head of the social networking service’s tooling shared a few learned lessons along the way.

Reading and reviewing code is something every engineer does on a daily basis. A formal code review process, however, is a bit different — it requires every code change to be officially reviewed by another team member before the code goes to production. At LinkedIn, code reviews have been a mandatory part of our development process since 2011. Our goal in requiring code reviews was to scale our rapidly-growing engineering team as smoothly as possible. Good code reviews with meaningful, useful comments can really help with leveling up an entire engineering organization. At LinkedIn, these reviews have become an essential part of quality assurance and knowledge sharing. Embracing code reviews has changed our whole engineering culture for the better in several key ways.

Szczepan Faber
Since 2015, Szczepan Faber has been a Tech Lead for LinkedIn Development Tools, the team responsible for tools and workflows that ensure engineering productivity. From 2011-2015, he was core engineer of Gradle 1.x and 2.x., a leading build and automation framework. Szczepan created Mockito framework in 2007, one of the most popular Java libraries, with an estimated user base of 2 million.

One of the biggest benefits of implementing company-wide code reviews has been increased standardization in our development workflow. Every team at LinkedIn uses the same tools and process for doing code reviews, which means that anyone can help with reviewing or contributing code for another team’s project. This eliminates problems like “I could fix the bug in their code, but how would I build that code and submit the fix?” This, in turn, helps increase collaboration across different teams in the engineering organization.

By making code reviews a mandatory process, we’ve also helped foster a healthy feedback culture at the company: engineers are open to giving and receiving feedback in all areas of work, not just in coding because it’s become a routine component of the job. Rather than viewing code reviews as critical or negative, our engineers use both giving and receiving code reviews as opportunities to grow professionally. In fact, high-quality code reviews are an important part of LinkedIn’s promotion process because they provide objective evidence of engineering skill.

Over the years, we’ve honed several best practices and tips for how to give truly great reviews. Below are some guidelines, in the form of questions, that we suggest asking to help make sure the reviewer and reviewee are getting the most value possible out of a code review.

Do I Understand the “Why”?

To facilitate the best review possible and help your team scale, every code change submission should include a design overview that briefly explains the motivation behind the change. It is really hard to offer a high-quality code review when the rationale needs to be inferred from the code change itself. It is fair to ask and expect the submitter to explain their motivation before attempting the code review. This also encourages the submitter to have an explanation in their commit message, increasing the quality of code documentation.

Am I Giving Positive Feedback?

In an organization full of smart people, clean code and neat test coverage can be taken for granted. As a result, code review feedback tends to focus only on problems and issues found in the code. This is very unfortunate because most people need positive feedback to feel engaged and motivated — and engineers are no exception. When a reviewer sees good stuff in the code, they should call it out and give positive feedback. This helps improve team dynamics, and often such positive feedback is contagious. As with all code review comments (more on this below), any positive feedback should be specific, explaining why that particular code is well-written.

Is My Code Review Comment Explained Well?

Whether the feedback is positive or negative, any code review comment should be self-explanatory. What might seem obvious to the reviewer can be unclear to an engineer who receives poorly-explained code review comments. When in doubt, it is better to over-explain than to provide terse feedback that yields more questions and the need for more back-and-forth communication. Explanations can be as simple as “reduces duplication,” “improves coverage,” or “makes code easier to test.” In addition to making reviewers’ comments clearer, these types of explanations also help reinforce the design principles that the team aspires to meet.

Do I Appreciate the Submitter’s Effort?

Hard work always needs to be appreciated, regardless of the outcome — this fosters strong, highly motivated teams. Some code changes are not of the highest quality and will need to be reworked. In those situations, it’s important to still acknowledge the effort that the author put into the changes, even if their code needs reworking. The best way to show appreciation is to put effort into your code review by giving high-quality feedback with decent explanations, acknowledging the good ideas (there are always good things in every code submission!), and using “thank you.”

Would This Review Comment Be Useful to Me?

Asking this question is an easy and powerful way to validate if the code review comment is necessary. At the end of the day, engineers should view code reviews as helpful development tools, and not sources of unimportant busywork. If you don’t think a particular review comment would be useful to you, then remove it. A classic example of unhelpful code review comments are ones related to code formatting. Code style and formatting should be validated by automated tools, not engineers.

Is the “Testing Done” Section Thorough Enough?

At LinkedIn, every code change submission has a mandatory “testing done” section that needs to be filled out. In the open source world, using GitHub as an example, engineers can submit “testing done” information in the pull request description. What should be in “testing done” depends on the gravity of the changes and the current level of test coverage. If the change contains new or altered conditional complexity, it is fair to expect it to be covered in unit tests. Some changes may require running manual testing if the integration test coverage is inadequate. In those cases, “testing done” should include information about the test scenarios and the outputs. When changes alter the output of the program, it is very useful to include the new output in the “testing done” section.

Am I Too Pedantic in My Review?

Some code reviews have so many comments that important issues — ones that really need to be fixed — are lost among less important suggestions. Reviews that are too heavy on details for a given team can slow down the review cycle and cause friction for both the reviewers and reviewee. Having clear review expectations, example reviews, and a positive, inviting review culture are great ways to avoid lengthy, exhausting review cycles.

In summary, having a formal code review process helps improve code quality, team learning, and knowledge sharing. The quality of work increases when every engineer on the team realizes two important things: someone else will read my code, so it better be good, and I’ll have to address any review comments I receive, so I should try to make my code good the first time around to save myself effort later on. When code reviews become an everyday habit, the team practices giving and receiving feedback on a daily basis. This is key for growth and improvement.

At LinkedIn, we’ve learned a lot from the past one million code reviews, and we aspire to learn even more from the next million. The more effort that’s put into code reviews, the better a team gets at giving great code reviews, the higher the quality of code that is checked in, and the higher the quality of the product that’s built. High-quality code reviews are contagious!

The author thanks James Miller, Oscar Bonilla, Joshua Olson, Andrew Macleod, Scott Meyer, and Deep Majumder for insightful feedback on this article.

LinkedIn’s parent company, Microsoft, is a sponsor of The New Stack.

Feature image by JJ Ying on Unsplash.

A digest of the week’s most important stories & analyses.

View / Add Comments