How to improve the code review process and increase collaboration across your team
Intro
In software development, collaboration is crucial. It speeds up value delivery and it’s a key component of a healthy team culture. Collaboration is particularly crucial during the PR review process, as that defines the quality of your value delivery.
This playbook will discuss how Flow can help you improve your PR and collaborative processes using the Review Collaboration report, and we’ll dip into Team Health Insights to see the bigger picture of just how much collaboration impacts team initiatives.
Customers who use Flow have decreased their time to merge by an average of 14 hours and they increased their iterated PRs by 37% on average.
For this guide, we’re focused on engineering teams that use PR's (like Trunk and GitFlow-based development workflows). However, these practices will also help teams who don’t use PRs, as the TL;DR version is to use data to make sure teams are working with each other constructively.
Determine your baseline
Before you adjust your collaborative process, you need to know where you’re starting. Use Flow to see how your team is currently working together, and then follow this playbook to make positive change.
-
Start with Team Health Insights.
- Select “Trailing 90 days” to get a sense of your team’s recent success.
- How quickly does your team respond to each other? And how long do they take to apply feedback?
- Click on the “Edit metrics” dropdown, and select all of your Culture metrics as well as iterated PRs and PR iteration time. This will give you averages, as well as visualize how each metric has changed across time and across teams.
- Go into Review Collaboration to look at who is collaborating with whom within your team. This helps you discover knowledge silos and make sure engineers with different skill sets and experience levels are learning from each other.
- These metrics are indicators of the collaborative process. They show when your teams are working together on their code, and how quickly they go through that collaborative process. Of course, there are other moments of collaboration, such as team meetings or quick chats. You’ll see these metrics improve as those other moments of collaboration improve, too.
Reviewing PRs
When looking at the PR review process, there are three metrics you’ll want to track. All of them are available in Team Health Insights.
Strike the right balance, here. Developers shouldn’t feel the need to drop everything when they get a request for a PR review, but they also shouldn’t ignore the request, which blocks their team members. We recommend shooting for an average of six hours for time to first comment. This means developers are responding within a day.
A review process is useless if it’s full of rubber stamping. While some PRs will only need a quick approval, others need thoughtful feedback. “Thoroughly Reviewed PRs” will help you strike this balance. We recommend a percentage of about 80% thoroughly reviewed PRs; that suggests your engineers are providing feedback when necessary, but they also know when a PR is simple enough that it just needs a quick approval.
Sometimes unreviewed PRs are a function of competing priorities, and they get stuck waiting for a review. Other times, unreviewed PRs are tickets that make it into production without a review, which is a security and quality risk.
Click here for a summary of code review best practices.
Next, use Review Collaboration to ensure that all of your engineers are collaborating with each other. We tend to spot three blockers:
- Unintentional bottlenecks. A manager or senior engineer feels the need to review all code. This causes engineers to miss their deadlines and feel frustrated by their team, while the rest of the team misses out on learning opportunities.
- Knowledge silos. This appears in two forms. First, one engineer is a pro in one language, so the rest of their team makes them review everything in that language. Alternatively, two engineers are friends, so they only review each other’s code. Both of these scenarios are full of good intent, but engineers should spread their knowledge as much as possible.
- Overburdened engineers. One engineer feels the pressure to get all their work done and review everyone else’s code. This causes annoyance and stress for everyone.
These three issues often create a negative cycle where one issue leads to another. For example, an engineer that knows one language better than the other engineers can create a knowledge silo and they’ll cause a bottleneck and they’ll start to feel overburdened.
To overcome these issues:
- Try to mix the seniority of engineers, and make sure that engineers with the right expertise are reviewing each other’s work. It’s a great learning experience to have a junior developer review a senior developer’s work.
- Check in on the people reviewing the most PRs; they may be contributing to the collaboration issues we discussed. Use Review Collaboration to spot potential bottlenecks, knowledge silos or overburdened engineers.
Iterating PRs
A review process is only good if the team makes changes based on the feedback. You’ll want to look at three metrics, here, too.
Responsiveness looks at the time it takes the original PR submitter to respond to their reviewer, with either a comment or a commit. A long responsiveness suggests that your submitter is getting distracted by other work, or they’re feeling stuck after receiving the reviewer’s response. If an individual frequently has long responsiveness times, have a conversation about what’s delaying them.
Iterated PRs can mean two things. They’re generally a good sign, as it means the PR process is working as intended; your developer received feedback that improved their code. This makes things more secure, decreases bugs, and improves UX. However, too high of an iterated PR percentage can suggest that people are over-relying on the review process—perhaps they're not engaging in thoughtful self-review, because they figure someone else will catch their mistakes.
Iteration time shows how long it takes a submitter to implement any changes to their PR. Again, a high iteration time could suggest that the feedback was complicated and not a simple fix. Or, the developer could be distracted by other work.
Merging PRs
The last part of the PR process is to merge. Time to merge will show your final timing for when the PR was opened, reviewed, and then merged. The faster the better, here, without compromising on quality. You can strike that balance by looking at efficiency, which is the percentage of contributed work that didn’t require rework later on.
Your ideal mix is a fast time to merge and a moderate efficiency percentage. A slow time to merge but high efficiency means your developers are doing good work, but perhaps getting slowed down by other work. And a fast time to merge but low efficiency means that developers are feeling too pressured to meet deadlines, so they’re missing out on quality. And a fast time to merge and high efficiency suggests that engineers aren’t going back to fix their work as often as they should.
The PR process is cyclical by nature, as is the collaboration between engineers. The more you can foster collaboration, the more successful your PR process will be, and the happier your engineers will feel. To improve other areas of your process, visit Flow Academy.