.tech Podcast - Alternatives to async code reviews

Blogs· 4min January 19, 2023

Dragan Stepanović is a Senior Principal Engineer at Talabat. He joins Renato Rodrigues de Araujo, Senior Software Engineer at Form3, to discuss asynchronous pull request based code reviews. Dragan shares a study he conducted on the topic and discusses the advantages of synchronous team collaboration.

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.

Introduction to async code reviews

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.

Async code review study

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.

Engineering team collaboration

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.

The cost of synchronous collaboration

 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.

The educational value of code reviews

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.

Written by

github-icongithub-icongithub-icon
Adelina Simion Technology Evangelist

Adelina is a polyglot engineer and developer relations professional, with a decade of technical experience at multiple startups in London. She started her career as a Java backend engineer, converted later to Go, and then transitioned to a full-time developer relations role. She has published multiple online courses about Go on the LinkedIn Learning platform, helping thousands of developers up-skill with Go. She has a passion for public speaking, having presented on cloud architectures at major European conferences. Adelina holds an MSc. Mathematical Modelling and Computing degree.

Further resources

Blogs · 5 min

Phishing with GitHub

For a Red Team operator it can be disappointing to retire a particular technique, but it can also be an opportunity to share their knowledge with the community. Phishing operations can require a lot of time and effort to set up the infrastructure, acquiring and categorising domains, fine tuning payloads, preparing pretexts and bypassing those pesky filters and controls, but there are ways to make the process simpler. This post will explore one such method, using GitHub as a tool to distribute, host, and compromise a target in a bait, hook, and catch operation that can be done from a mobile device. This post will cover: GitHub Apps, Hosting, Distribution and SSH Access.

February 1, 2023

Blogs · 5 min

My first month as a Senior Engineer at Form3

It's always daunting moving jobs. In this post, Chris Townsend shares insights into his first month as a Senior Software Engineer at Form3. He talks us through his reasons for joining, the interview process and his onboarding experience, as well as what his future career aspirations are.

January 25, 2023

Blogs · 6 min

Adventures into Electron code injection on MacOS

Process injection in MacOS is a difficult topic: it is well controlled and there are simply no API calls that provide any useful interface for it. As it is a feature that rarely has legitimate use cases, it makes sense from a security perspective to disable it entirely, or at least heavily restrict it under normal user conditions. However, as a red teamer, it is difficult to move from the freedom of process hollowing and remote threads on Windows, to the harsh reality of the MacOS hardened runtime. This is true especially when trying to create hidden C2 channels and evade detection from EDR and XDR software. There is one technique, however, that does not get the recognition it deserves, most probably because it can only target Electron based applications. While this sounds like a big limitation, there are popular applications that can be targeted and are more than likely to be present on the target system such as Slack, Visual Studio Code and Microsoft Teams to only name a few. These applications can all be a target of code injection by abusing Electron's built in remote debug interface.

January 11, 2023