.tech Podcast - Alternatives to async code reviews

Dragan Stepanović is a Senior Principal Engineer at Talabat. Dragan has experience working at different sizes of companies, from small to large corporates. He became interested in Extreme Programming (XP) early on in his career. Then, he started diving into architecture, Domain Driven Design (DDD) and LEAN as tools to enable engineers to maximise their throughput for their stakeholders.

Renato Rodrigues de Araujo is a Senior Software Engineer at Form3. Renato is part of the Tooling Team, which is responsible for making the lives of engineers easier by maintaining internal tools.

Dragan explains that async code reviews are a way of working that is closely related to the pull request (PR) based model.

The process usually consists of:

• At the start of the sprint, one developer starts working on a feature, writing the code and tests.
• Once they are satisfied, they invite other engineers in their team to give them feedback.
• The developer then raises a PR with the changes that they've made and invite their team mates.
• Those invited to give feedback are not usually available immediately, so the review typically does not happen immediately.
• As the author waits for their team feedback, they typically start working on another feature or piece of work.
• The review process goes back and forth between the author and their team with a delay.
• The async code review process inherently involves delays in the process, as reviewers ask for changes and authors then have to incorporate them later when time allows.
• At the end, the PR becomes approved and the change becomes incorporated in the main/trunk branch.

The pull request based async code review model of working has been adopted from the open source community, which tends to be a very different context than we have in typical product development teams. This is now the most popular way of delivering features.

Renato shares that Form3 uses a mixture of practices due to the nature of the business. As Form3 deals with sensitive financial data, some changes require verification by specialists outside the team. The teams do maintain their own repositories and approve as many changes internally as possible. For time sensitive changes, teams use pairing and mobbing sessions to accelerate the development and review process, mitigationg some of the delays usually incurred by async code reviews.

Driven by a want to help teams improve their ways of working, Dragan has conducted a study on async code reviews. Based on his experience with extreme programming which involves a lot of collaboration, he wanted to see how he could improve their development process and shorten delays.

The study consists of analysing more than 40000 PRs in over 40 very active repositories in typical product development teams. The significant study highlighted some interesting insights. One of the most surprising findings is around the delays incurred by PRs.

The queue time or wait time represents the time that the pull request stays in an open state, not being worked on or reviewed. The study plotted the correlation of PR size to wait time in a scatterplot. The systemic behaviour that emerged showed that the wait time per size increases exponentially.

Developers generally agree that big PRs are hard to understand and review and they should be avoided. However, keeping the PRs small improves the readability and review difficulty, but incurs exponentially long wait time per size, which is introduced by the delays of the async code reviews on multiple small PRs. This means that the system throughput also exponentially decreases. In a nutshell, small PRs actually increased delivery times when used together with async code reviews. This is definitely a surprising finding of the study.

Dragan shares that teams can easily collect these metrics in their own repositories by looking at the lead times of their PRs, which is time elapsed from the first commit to the time that the PR is merged. They can then plot these times against the size of the PR to gain insights into their ways of working.

Based on his extensive experience, Dragan can make some recommendations on team collaboration, aside from the imperfect practice of using async code reviews:

• Teams should shift the things that they optimise for and focus on flow efficiency, as opposed to resource efficiency. This change in focus will help teams optimise the lead times for customer delivery, as opposed to measuring how much work the individual developer is able to deliver. The concept of flow efficiency comes from the LEAN way of working.
• As a progressive idea from async code reviews to more synchronous work, moving to a continuous code review helps to diminish the tradeoff between speed and quality. Teams should try to review PRs synchronously, as soon as the author raises it.
• The next step from synchronous code reviews is then to collaborate and make changes together. If you have everyone that you need working on the change, then you also have the expertise to review the change present as well.
• Teams should also set the expectations that they will focus on shortening lead times and improving the process of delivering customer value. This calls for changes in engineering culture, where engineers know that they will be working together. Everyone in the team should optimise for the same things to collaborate synchronously and successfully.

Remote working can make synchronous collaboration more difficult due to the difference in timezones. Renato shares how synchronous collaboration happens in his team, which has colleagues based in the UK and Canada. The 5 hours difference between the two main locations means that synchronous collaboration can be more difficult. Renato believes that working together is not only about coding together, but also understanding and breaking down the problem together. Long hours of calls can be difficult to manage, but getting the team on the same page as early as possible will help minimise the number of times that the PR review goes back and forth, thus reducing the delay incurred by async code reviews.

 Dragan addresses the myth that synchronous collaboration increases costs. If we want to reduce cost, we should focus on improving the throughput of our teams, which reduces costs as a byproduct.

Multiple people working on the same project or domain are heavily interdependent on each other. In such a team, if people are working individually, we are sure that we will incur delays. Therefore, it pays to have a systemic view in mind, as opposed to focusing on what the individual people are working on.

Renato uses pairing in his team and can offer the perspective of a Form3 team. Due to the reliance on specialists, some pieces of work still incur delays due to the need for approvals, even if the team is pairing. However, if the team has full control of its work, then pairing does speed up development.

Code reviews are often seen as an educational tool. However, Dragan points out that they do not tell the story of how the developer arrived to the solution. The just-in-time, immediate feedback received through pairing and mobbing already contains this context.

Therefore, synchronous collaboration accelerates the rate of knowledge sharing in the team, especially when it comes to internal team silos. This also provides a lot of flow resilience when it comes to services, improving the mean time to recovery.