Originally published on the Ness Digital Engineering blog at https://www.ness.com/team-getting-code-reviews/
Most serious developer teams today have at least some form of code review discipline in place. This means that at least one person, other than the author, looks through a new code change before the change goes out to production. There is a good reason for that; code reviews, even when practiced less formally, have been found to increase the quality of a software product by reducing the number of defects. See [1] for research involving modern open-source software projects.
As principal engineer at Ness Digital Engineering, I see teams that do code review with varying degrees of rigor, from a light touch approach, all the way to mandatory, thorough reviews by at least two other engineers.
Irrespective of the actual process, the benefits of a code review discipline in your team go beyond whether the number of coding defects is below your threshold for quality. We will take a look at those first.
What I’ve repeatedly noticed is that very few engineers actually look forward to having their code reviewed. It feels like a trip to the dentist: you know it’s good for you, you know that a small intervention now prevents major decay later, but still your mom or your partner has to push you to go, because the anticipation of the drill in your mouth doesn’t exactly thrill you. In the second part of this post we’ll review some tips for running effective code reviews.
Here are other benefits of code reviews you should keep in mind for your team.
1) Code reviews spread knowledge and create a sense of ownership of the entire code base. If Alice has become an expert in module A of your code base, and Bob is an expert in module B, Alice can learn new things by reviewing Bob’s code changes in module B, and can in turn, guide Bob when he makes changes in module A. Your team is healthier when it’s free of “cliques,” and everyone feels that it’s OK to make changes to any part of the code. Also, most assume that it’s always the more senior engineer who reviews the junior engineer’s code, but a junior engineer should also be asked to do a review; he or she will learn a lot by reading through and trying to understand a code change from a more experienced colleague. Oh, and, inevitably, the time will come when a junior finds an issue with a senior’s code. To find out what to do to prevent bruised egos, skip towards the end of this post.
2) Code reviews add skin in the game. If I review and approve a code change by Charlie, my name is on the record. If that change introduces a defect, it will not be just Charlie who takes the heat.
3) Code reviews prevent deviation. It’s important for a team’s code base to have a consistent “style of doing things” everywhere. A consistent style reduces the cognitive burden of an engineer who must make changes to an unfamiliar section of the code. It doesn’t have to be the best way of doing things, even when the best way is objectively so.(it’s usually not, and likely to cause endless debate within the team). Here is an exaggerated example of why consistency is important. You can debate whether it’s better for motor vehicles to drive on the left or on the right side of the road, but an “interim” state, when trucks drive on the right and small cars drive on the left, would be a disaster.
Here are some tips on how to run code reviews effectively and with minimum pain.
- Make them quick and non-formal. There should be no extensive meetings, and the process should be as light as possible. This lessens the friction for the reviewer. The easier it is for a reviewer to jump in, the more time he or she will have for looking over the code changes, and that’s a good thing. [1].
- Make them frequent. Discourage huge changes and instead, insist that changes be submitted, reviewed and merged after two days of work, maximum.
- Make them painless for the reviewee too. Encourage the reviewers to critique the code verbally in a one-on-one with the author. Discussions will be faster and less confrontational when done face to face. Beware of the cultural sensitivities in your team. Some authors may feel uncomfortable or ashamed if there are permanent records of the shortcomings of their code, such as normally done in a GitHub pull request. In that case, it’s better that all reviews happen verbally, and the only permanent record is the sign-off (thumbs-up) from the reviewer.
- Give adequate time for the reviewer to actually read and understand the code change. Common sense, as well as research, [3] indicates that the effectiveness of reviews is reduced by the more lines of code per hour a reviewer is asked to review.
- Make the reviews fair. Ensure reviews are not dictated by the personal taste of the reviewer. Instead, insist that the team have a Coding Conventions document.
- Review with an optimistic mindset. If the reviewer can’t find anything wrong with the code, it’s probably because the code is fine and should be given a thumbs-up right away. Don’t guilt-trip reviewers into hunting for issues where there aren’t any. The review activity is sure to descend into nitpicking. Anyone who has been on the receiving end of it remembers the frustration. This is a toxic culture that should not be allowed to spread in your team. Keep the reviewers focused on major functional issues, or issues affecting readability and changeability, such as proper naming and adherence to SOLID principles. Don’t focus on line length or any sort of issue that a static analyzer can catch. [2] In fact, you should have a static analyzer (such as ESLint or Sonar) run before starting a code review.
- Finally, just like with any process, don’t let the code review process go stale. Take the opportunity to improve things after every sprint retrospective. Measure the number of reviews per change, the total time a task spent in code review, and the number and magnitude of new commits added to the changes during review. This gives you an indication of the impact of the reviewers on the code being reviewed.
If your team is just getting started with code reviews, you will probably need to choose a tool. Pick one that the team already knows or already has installed. Your code hosting platform, e.g. GitHub, BitBucket, Gitlab, already offers code review features. There are also open-source tools specifically for code review, e.g. Gerrit, Phabricator Differential, ReviewBoard, as well as many commercial offerings.
[1] McIntosh, Kamei, Adams, and Hassan, The Impact of Code Review Coverage and Code Review Participation on Software Quality, http://sail.cs.queensu.ca/Downloads/MSR2014_TheImpactOfCodeReviewCoverageAndCodeReviewParticipationOnSoftwareQuality_ACaseStudyOfTheQt,VTK,AndITKProjects.pdf
[2] Bosu, Greiler, and Bird, Characteristics of Useful Code Reviews: An Empirical Study at Microsoft, https://www.microsoft.com/en-us/research/publication/characteristics-of-useful-code-reviews-an-empirical-study-at-microsoft/
[3] Kemerer and Paulk, The Impact of Design and Code Reviews on Software Quality: An Empirical Study Based on PSP Data, http://www.pitt.edu/~ckemerer/PSP_Data.pdf
Author: Iulian Dogariu, Ness Digital Engineering