The engineering manager's guide to code review

Updated on December 1, 2022 | Read time: ~15 mins

The general purpose of code review is for development teams to recognize and remedy bugs before code hits production. Yet we also believe that code review can accomplish much more than just that one pragmatic outcome when leaders and managers become meaningful participants in the review process.

In this guide, we discuss nine review dynamics common to engineering teams to help software engineering leaders improve collaboration and problem-solving. We also describe how to use Pluralsight Flow to track these dynamics.

1. Overlooking the value of code review
2. Long-running PRs
3. Heroing
4. Over-helping
5. Over-communicating
6. Just one more thing
7. Knowledge silos 
8. Self-merging PRs
9. Ramping up
   
   Conclusion
   

Beyond fixing bugs, code review results in higher quality code that is more broadly understood across a team. Studies show that this process saves money, reduces reliance on QA, and improves engineering development, knowledge sharing, and the overall culture of the team in addition to the quality of the code. 

It’s also an opportunity for engineers to collaborate, learn from their peers, practice mentorship, and discover improved solutions to problems. 

Leveraging code review prior to production streamlines the development process, highlights bottlenecks and process issues, and sets the stage for improving the overall health and capacity of teams.

“Code review has been shown to decrease errors by a staggering 80%.”

Even as recently as a decade ago, the primary (and often sole) purpose of conducting code review was to ensure the quality of the code. There are different ways of doing so, of course. But none of them can compare to peer review.

In Code Complete: A Practical Handbook of Software Construction, author Steve McConnell identifies that the average defect detection rate for unit testing is a mere 25%, with function testing’s rate at 35% and integration testing at 45%. Likewise, he states that the average effectiveness of design inspections is 55%, and 60% for code inspections.

Code review has been shown to decrease errors by a staggering 80%. McConnell points to a study that examined a group of 11 programs developed by the same team—five of which underwent no review at all. Those five programs contained, on average, 4.5 errors per 100 lines of code, while, the six programs that underwent code review averaged only .82 errors per 100 lines.

Some notable companies have shared research that indicates the use of review as a vital component of saving money and reducing errors:

• IBM’s 500,000-line Orbit project used 11 levels of inspection and had only about 1% of the errors that would be expected without review—all while achieving early delivery.

• A study of a 200-person organization within AT&T introduced code reviews and subsequently experienced a 14% increase in productivity and a 90% decrease in defects.

• Jet Propulsion Laboratories estimates savings of approximately $25,000, simply by identifying (and repairing) defects in review at earlier stages.

A large-scale study of coder review coverage and participation at the University of California-Berkeley shows that code review reduces the number of bugs and security bugs in production. The research also suggests that implementing code review policies may have a positive effect on the quality and security of software.

Researchers conducted a survey with 645 top contributors to active OSS projects. The results suggested that engineers have a strong interest in maintaining awareness of project status to get inspiration and avoid duplicating work—but they don’t tend to proactively propagate information. In other words, there’s often a delta between how engineers want to leverage the review process and how it’s actually being leveraged. 

The study pointed out that the challenges that are causing these barriers that limit the outcomes of code review are mostly social in nature. This presents a clear opportunity for managers to facilitate discussions in retrospectives or standups about the “state of the code review process” within that team, and how it might be improved.

Furthermore, code review helps organizations become more resilient and adept by managing and reducing knowledge silos. Atlassian’s guide to agile development contends that agile teams realize such benefits when code review decentralizes work across the engineering organization. Specific knowledge about the code base is not exclusive only to one team or even one engineer.

Ultimately, the job of engineering leaders is to remove obstacles so their teams are able to spend more time working on valuable solutions and so their work output has the reach, impact, and visibility it deserves. So it would be easy enough to designate code review as the engineers’ domain, where managers need not tread.

Yet where training was once at the heart of management, code review is now the prime way of improving an engineering team’s output. By participating in and observing code review, managers are able to track the health and productivity of the team, which provides insight into where to intervene and where to encourage progress.

Here, we’ve assembled eight of those dynamics that demonstrate the behaviors common to developers and to engineering teams, how to recognize them using Flow metrics, and what managers can do to bolster their teams’ health and productivity.

Long-running PRs can quickly be identified in the team’s Review Workflow report, filtered by “PR Status: Open” and sorted by “oldest PRs.” Select the number of PRs you’d like to see in one view, then hover over those that have been open for more than a day.

If you see a few back-and-forth comments with signs of uncertainty or disagreement in the communication, followed by silence, it’s worth checking in to see how you can move the conversation forward.

• If there are signs of disagreement or confusion in the discussion: Check in with the Submitter. If there is a disagreement, listen to their perspective and offer advice to move it forward. Depending on the situation, get the Reviewer’s read on it as well — ideally when everyone is together in a room or on a call. Make a decision, and ask anyone who disagrees to “disagree and commit” for the sake of the team’s progress.

• If there are large spaces between comments and responses in the review: Set expectations or targets around Time to First Comment and Time to Resolve. (Both metrics can be found in Flow’s PR Resolution report.) It can also be helpful to communicate best practices around responding to colleagues in a timely manner. 

Heroing is the tendency to fix other people’s work at the last minute. Right before each release, the Hero finds some critical defect and makes a diving catch to save the day.

Of course, attention to detail is essential and a good save is usually better than no save. But regular Heroing leads to the creation of unhealthy dynamics within the team. It will ultimately short-circuit feedback loops and, over time, can foster uncertainty and self-doubt in otherwise strong engineers.

You’ll be able to note heroes with Flow’s Help Others metric, particularly in the form of late arriving check-ins. They’re also distinguishable in the review process, where they may be self-merging PRs (and typically right before the deadline), or they will show very low Receptiveness in the review process (meaning either others aren’t providing substantial feedback or the Hero isn’t incorporating it).

• Rather than managing the “saves,” manage the code review process. Ideally, team members are making small and frequent commits and requesting interim reviews for larger projects. 

• When the team is in the habit of getting feedback early and often throughout a project, as opposed to submitting massive PRs all at once, the barrier to participating in the review process is lower. This can make it easier to promote healthier collaboration patterns and get everyone to give and be receptive to feedback in reviews. 

• Coach the Hero to turn their “fixes” into actionable feedback for their teammates to implement with time to spare.

Collaboration among teammates is to be expected, as it is a natural part of the development process. However, “Over-helping” can occur if one developer spends an unnatural amount of time helping another developer get their work across the line.

The problem is threefold: (1) always cleaning someone else’s work takes away from one’s own assignments, (2) it impairs the original author’s efforts toward true independent mastery, (3) it can overburden the helper and leave the original author in a continuous unnatural waiting state.

You’ll notice this common dynamic in the same way you’d realize Heroing in Flow’s Review Collaboration report and the Help Others metric. Look for reoccurring, last-minute corrections between the same two people.

This behavior can be perfectly healthy and expected when in a mentorship-type situation. But beyond a certain point, rotation is in order.

• Bring additional engineers into the code review process. A side effect of this solution is that by increasing the distribution of reviews, you’re strengthening the team’s overall knowledge of the codebase.

• Cross-train and assign both engineers to different areas of the codebase.

• Assign the senior engineer a very challenging project. The idea here is to give them challenging projects where they don’t have the time or energy to review their colleague’s work.

• Lastly, the stronger of the two is showing natural leadership and coaching tendencies. Look for opportunities to feed these skills more broadly to the whole team.

It may seem counterintuitive that over-communicating in code review can be a problem. The problem arises when over-engaged engineers spend a disproportionate amount of their time providing feedback (helpful or not) on other people’s work and too little time working on their own projects. The healthy mix of reviewing and contributing is thrown out of whack.

You may first spot this dynamic in Review Workflow when you notice a team member is frequently engaging in the PR process as a Reviewer. This behavior is only something worth noticing when the feedback that’s being provided is going far beyond what’s “good enough” for that specific project. 

If you notice a highly engaged Reviewer, you can go to their Player Card to see their level of Involvement and Influence—and evaluate that in context with their Code Fundamentals (“Is this team member spending more time in review than on their own work, and is that expected?”), and with the team’s averages for that time period.

• Over-engaged engineers may have come to believe through learned behavior that commenting in this way allows them to contribute most helpfully to the team. So as a leader, you need to ensure that you’re setting proper expectations and guiding these engineers to correct course.

• Add projects to their bucket of work over the next couple sprints. As they feel the pressure of hitting their deliverables, they’ll naturally tend to scale back on their commenting activity. 

• By doing this, you can let them know, “You clearly have a lot of work on your plate. If that needs to come at the expense of some of the code review you’ve been doing, that’s totally okay.” Just giving them that little out is usually all you need to restore balance on the team.

Just One More Thing refers to a pattern of late-arriving pull requests. A team submits work, but then—right before the deadline—they jump in and make additions to that work.This pattern can occur for a wide range of reasons, including last minute requests, poor planning or estimates, and too much work in progress.

Just One More Thing, when appearing across a team, is characterized by a spike in PRs being submitted near the end of a sprint after the main PR was approved. These engineers will also show a high level of New Work.

• When you notice a spike in PRs being submitted, it can be helpful to review the work submitted and decide whether it should be given an extra day’s review.

• In the long run, consider working with the team to identify any bottlenecks or process issues that could be eliminated or improved.

• If the team’s estimates or deadlines are causing last-minute stress, consider setting different internal deadlines for projects. 

• Another framework that some teams use is to consider the three levers in setting a deadline: the external deadline (if any), the scope of the project, and the resources available. 

• If last-minute requests are coming in from outside the team, talking to the stakeholders or managers whose groups are regularly causing the problem can give you the opportunity to show the impact of the problem and understand what’s going on from their perspective.

Knowledge Silos form when a group of engineers review only each others’ work. There are plenty of reasons why engineers will get into a cycle of reviewing only each other’s work — figuring out the reasons why, through discussions with the team and by reviewing the Team Collaboration metrics, can sometimes point you toward the broader team dynamics at play. 

Whatever the cause, reviewing a select group of engineers’ work for a long time can lead to less substantial reviews simply because the engineers trust that each others’ work is good enough. When that happens, these situations can turn into bug factories. Work is being approved and pushed forward without adequate evaluation.

You can also use the Knowledge Sharing report to visualize how knowledge is being distributed across a team in the review process and to identify knowledge silos. If there are two or three people who review only each others’ code, the team’s Knowledge Sharing Index will trend toward 0. If the majority of the team reviews each others’ code, the Index will trend toward 1.

You can then drill down into specific team dynamics with the Review Radar. When there are silos, there will be a small group of engineers who review only each others’ work across multiple sprints.

• Bring in the outsiders! Get other engineers involved in the review process. You can also see whether there’s anyone who could be cross-trained or onboarded on a specific area of the code that an engineering within the silo is working on.

• Have the individuals within that tight-knit group review the work of others outside their group.

Self-merging pull requests refers to when engineers open a pull request and then approve it themselves. This means no one else reviewed the work and it’s headed to production.

As a general rule, engineers should never merge their own code. In fact, most companies don’t permit them to: self-merging bypasses any form of check on the code, as well as skipping the opportunity for improvement and learning.

Self-merging is easy to see because the submitter and the reviewer are the same people. In Flow, these instances will show up in the team’s Unreviewed PRs metric as well as in the Review Workflow.

• Many organizations prevent self-merging PRs by configuring their build systems to reject them. Enforced review is most common among companies that work under regulatory compliance, like Fintech or Biotech companies.

• Even in organizations that don’t enforce review, reviewing these PRs on a case-by-case basis after they’ve have been merged will help ensure that any bugs or problems are not going to get buried.

• If the commit was trivial, you might be able to give QA a heads-up to take a close look at it. If the unreviewed pull requests are non-trivial, walk those back if the circumstances allow and require a code review.

• If engineers are in the habit of self-merging without review, it may be helpful to have an informal conversation with them to ensure that they understand the why behind the review process or that they are at least clear on expectations.

Code review is always a chance for cross-pollinating information and expertise between team members. So using PRs in the process of onboarding and ramping up engineers new to the organization is a great way to build connections between developers who will be working together.

• Use the data to understand the dynamics of a new hire’s review engagement. By looking at PRs Submitted and PRs Reviewed, you can identify where new hires are interacting with their teammates. After all, commenting on multiple people’s work will build rapport within the team over time.

• The data can also help you understand the quality of their comments in the review process by examining the responsiveness and review coverage of both the new hires and the experienced developers. This will demonstrate the traction the new hires are getting within the team.

• By encouraging engineers to participate more in the PR process, not only are you encouraging best practices, you’re also empowering them to speak their minds and use their voices. “You were brought onto the team for good reasons,” you’re saying. “Your team wants to hear what you have to contribute.”

Managers can provide high-value contributions by looking at the code review process as a whole and noticing quick wins and areas where the team could work better together. A leader’s role is to remove obstacles that block developers from doing their best work, and to coach team members toward healthy work and collaborate patterns. They work on the process, rather than in the process.

We hope this guide helps deepen your understanding of the practical benefits of code review, as well as the manager’s capacity to support the team in reaching their potential. If you’re interested, there’s more to learn about how Pluralsight’s Flow can help you optimize your software delivery process with better tools for code review.