/
Improving Code Review

Improving Code Review

In November, the Escalation team brought down our average code review time for our tickets from around 6 days to around 3:

Along the way, we learned to work more efficiently as a team and began to feel empowered to continue improving our internal processes. We share this post not just to highlight our success in cutting down code review time, but also to show how we adopted and began implementing a new philosophy of internal process improvement.

The new idea

Before we were even thinking about code review, we came up with a hypothesis that the people who are facing blockers are the ones that are most well-equipped to come up with solutions to remove them. Before, we were operating under the idea that the team lead’s (Adam’s) responsibility was to focus on blockers and other logistics, so that the team could focus on their work. The unintended effect of that was the team was discouraged from thinking about solutions to the problems they actually faced, and the team lead did not have the full story of the problems he was trying to fix. If we were willing to invest some of the team’s time into process improvement, we might see better results.

In the spirit of experimentation, we decided to try it out!

Improving code review

At this point, we still hadn’t settled on a plan to improve code review. We just had our hypothesis that the team should be the one that focuses on blockers, not just the team lead. In that spirit, instead of mandating a project for the team to take up, we decided to venture a few options and vote. A brainstorming session and a google form later, we arrived at our project: improving code review.

Pain points

Once we had our goal—to reduce the pain of code review—the team gathered to identify the key pain points:

  • Un-Groomed Issues: Escalation often works on issues that are immediately frustrating users, so we don’t always have the luxury to groom every issue before working on its fix. There’s no way around that. But sometimes significant changes to a pull request’s approach were requested during the code review, creating rework.

  • Not-Well-Documented PRs: Previously, pull request descriptions explained the existing problem, but didn’t describe the fix that the pull request implemented. So every reviewer was spending extra time trying to understand the logic of the fix from the code changes themselves. Reviewers would get confused about the fix, causing them to ask clarifying questions that held up the review process.

  • 12-hours gap among reviewer and reviewee: Whenever an Escalation Lahore team member nominated someone in Cambridge for a review, the review was inevitably delayed by the time zone gap between the two locations. If the Lahore team submitted a pull request for review during Lahore hours, it would take half a day for someone in Cambridge to look at it during their working hours, by which time the Lahore team member would be asleep. By the time the Lahore team member could respond to the Cambridge team member’s comments, the Cambridge team member would probably be asleep. In that way, one small misunderstanding could set the process back a whole day.

  • Lack of motivation for Review: Lahore team members felt like they had to wait for someone in Cambridge to review a pull request before adding their own review. To increase their chance of getting a review, the Lahore team members would tag four or more people to review their pull requests, creating a tragedy of the commons scenario where each person tagged assumed one of the other tagged people would do the review.

Responses

To address these concerns, the team talked to other teams in Lahore about their processes, then came up with this this list of guidelines, which I’ll summarize here:

  • Spend some time grooming existing tickets

  • Try to keep reviews as internal as possible; if tagging someone in Cambridge, make sure you have an internal review first

  • If you’re a code reviewer, take the review seriously. Ask questions if you’re confused and find another reviewer if you’re unable to review.

  • Embrace a philosophy of “making it count”: our goal is to get fixes out the door

  • Create a sandbox for each pull request with instructions to test on it, if applicable

  • Use of PR template: we followed the TNL team’s lead and created a template for when you’re submitting a PR. This will provide the necessary context for reviewers to do more efficient and speedy reviews.

  • Communicate! Use hipchat, skype, or tap someone on the shoulder.

What we got out of it

We certainly improved our code review process, but we got a lot else out of the process as well:

  • Give team ownership: One of the first things we did was hand over our daily standup meeting to the remote team to facilitate. While a small change in our procedure, it made the remote team start to feel that they were as empowered as the team in Cambridge. After managing the stand-up meeting, team felt confident enough to take ownership of improving code review. One of the most important insights to come out of all this is that the team didn’t feel empowered before, either about the codebase or our internal processes.

  • Code Review is as important as coding: we had to accept that code review is as important as writing code. If you do not review, the change will not be merged and ultimately it makes no sense to write some code if it’s never deployed.

  • Feel confident: You (team) are owner of your code, feel confident reviewing your own code.

  • Performance Review Meeting: With our newfound confidence and sense of empowerment, we started looking internally about our own performance. How much work we were actually doing? How many issues were we triaging? How many issues are we fixing? Closing without a fix? The team decided to start capturing these metrics on a biweekly basis.

  • Embrace positive change

  • Be communicative internally

What we’ll do differently next time

We are currently employing the same process for a different self-selected process improvement. This time, we plan to improve it in the following ways:

  • During the brainstorming phase, some people felt left out, so we should experiment with other ways of brainstorming (like taking a  few minutes to write independently on sticky notes)

  • Assign an explicit Project Manager

Conclusion

The main benefit from the project wasn’t just the improvement in code review; it was the feeling of ownership and empowerment that the team felt in taking on the project in the manner that we did. Now, we’d love to hear your thoughts. Do you have suggestions for us? Did we have insights that your team might be able to use? Anything we overlooked or should change?

Thanks for reading!

Attribution

Original Author: Awais Jibran (Deactivated)

Edited by: Adam Palay (Deactivated)

Related content

A Data-Driven Productivity Gain on the Escalation Team
A Data-Driven Productivity Gain on the Escalation Team
More like this
Bi-Weekly Performance Review
Bi-Weekly Performance Review
More like this
Arch Tea Time: 2020-07-23
Arch Tea Time: 2020-07-23
More like this
Single Responsible Person Principle (for bugs & PRs)
Single Responsible Person Principle (for bugs & PRs)
More like this
2023-02-17 Maintainers' Office Hours Notes
2023-02-17 Maintainers' Office Hours Notes
More like this
2022-11-17 Maintainers Meeting notes
2022-11-17 Maintainers Meeting notes
More like this