There are different types of Code Reviews with different objectives. This article only refers to those Code Reviews that another team member does before incorporating the change in the shared code repository.
The objective of these reviews is to improve the quality, avoid errors introduced in the shared code repository, and prevent issues from going out to production.
Therefore the code reviews we are talking about:
- Avoid errors/bugs by acting as a safeguard.
- Occur within the flow of making changes to production.
In this blog post, we will not discuss other types of Code Reviews that are done outside of the normal development flow.
Most common: Asynchronous code reviews.
In this context, a very common (and for some, recommended) way of performing code reviews is that a developer (A) works individually on a task (d) and upon completion of the general change, creates a Pull Request (PR)/ Merge Request (MR) that another person in the team must review asynchronously.
We can see an example of this way of working in the following diagram. In it, we can see how developer A generates PR/MRs that developer B reviews, generating proposals for changes that developer A should incorporate.
In the example, the feature/change is composed of increments d1, d2, d3, and we see that we deploy it to production at the end of the code review of increment d3.
We can see that A's development flow is constantly interrupted to wait for the corresponding feedback for each code review. This way of working delays the release to production, increasing the time it takes to receive real feedback from customers.
Because each individual code review takes so long, there is an unconscious and natural tendency for developers to work in increasing larger increments. At least, this is what I found in many teams I have worked with.
It is a curious situation because, on the one hand, the industry says that working in small safe steps is a recommended practice. On the other hand, many companies use async reviews that tend to discourage them.
So the most common way of working looks like the following diagram:
Moreover, in this case, by generating large PRs/MRs, the code review loses meaning and, in many cases, becomes a pantomime (LGTM). (See: how we write/review code in big tech companies)
With this last way of working, the lead time to production improves a little, but at the cost of taking bigger steps and losing the benefits of detailed code reviews.
Small PRs tend to generate a lot of churn, people commenting etc. while bloated PRs just sail through -- no one wants to touch them with a 10 foot pole so they just push the Approve button.
— Alex Bunardzic, white non-colonizer (@alexbunardzic) July 21, 2021
When working with asynchronous code reviews, the reality is usually more complex than what we have seen in the previous diagrams. The reality usually includes many developers, simultaneous code reviews and cross flows in which there is a strong tendency for individuals to work alone and do continuous context switching.
The problem are not branches or PRs per se, but the async, and thus delayed code review that chockes the flow of changes through the system.#continuouscodereview
— Dragan Stepanović (@d_stepanovic) August 4, 2021
Is there a better way?
Fortunately, there are synchronous code reviews and continuous code reviews.Synchronous, high priority code reviews
The first step is to give the highest priority to code reviews and to do them immediately and synchronously between the person who developed the increment and the person in charge of doing the review.This way of working reduces the total lead time of each increment and encourages working in small increments.
As a side effect, the code reviews should be more straightforward because the developer themself explains them and can express in detail why they have taken each of the decisions.
This way of working requires a lot of coordination between team members, and it is not easy to make them always synchronous, but of course, it is a simple step if we are already doing asynchronous code reviews.
It is as simple as making PRs/MRs top priority, and as soon as you have one ready, coordinate with someone else to review it together on a single computer.
Pairing/Ensemble programming: Continuous Code Review
Pair programming: "All code to be sent into production is created by two people working together at a single computer. Pair programming increases software quality without impacting time to deliver. It is counter intuitive, but 2 people working at a single computer will add as much functionality as two working separately except that it will be much higher in quality. With increased quality comes big savings later in the project.". http://www.extremeprogramming.org/rules/pair.html
When we work using pair programming, as recommended in extreme programming, two people design and develop the code. This way of working generates a continuous review of the code, so it no longer requires a final review phase.
Of course, from the point of view of coordination, it is the simplest system, since once the pairs have been formed, each pair organizes itself to make increments, which are implicitly reviewed continuously.
In this case, the diagram representing this way of working would be:
A new way of working known as mob/ensemble programming has appeared as an evolution of the pair programming practice. In this modality, the whole team works simultaneously on a single task and using a single computer.
The whole team collaborates in the design and development, limiting the context switching and minimizing the lead time for the task. As in pair programming, mob/ensemble programming performs a continuous and synchronous review of the code.
Of the ways of working that we have seen, pair or group programming has the shortest lead time for each change, fewer context switching interruptions, and requires less coordination.
Even so, pair and group programming is not a simple practice and requires effort and a lot of practice. IMHO it is clear that this way of working takes the quality benefits usually associated with code reviews to the extreme.
The industry vision, my personal vision
The industry seems to agree that code reviews are a good practice and should be implemented by all development teams.
On the other hand, it is also recommended in most cases to work in small increments so that the complexity and risk of each change can be better controlled.
With these two points in mind, it seems a good idea to use Code Reviews and make them as synchronous and continuous as possible to work on those small increments of low complexity and low risk.
In my case, I have been successfully pushing pair or mob programming for a few years now, creating a stream of small increments that are reviewed continuously.
In both Alea Soluciones and TheMotion, we used pair programming by default and mob programming occasionally. Both companies' deployment frequency was daily, and we could make considerable changes in small and safe steps. In addition to this constant and sustainable development flow, we got attractive additional benefits such as the ease of spreading domain and technical knowledge or make fast onboarding of new members. In the case of Alea Soluciones, we hired people who already had experience in pairing and XP practices, so it was easier. In the case of TheMotion, however, we had to introduce some of the agile development practices (pairing, TDD, etc.), so we hired technical coaching help.
In Nextail there was a mix of teams doing pairing and continuous reviews and others using asynchronous code reviews but prioritizing to avoid queues.
At Clarity AI, some teams use asynchronous Code Reviews but prioritize them to minimize the lead time. There are already two teams doing continuous code reviews (using pairing or mob programming) and we are gradually expanding this way of working to other teams.
Of course, the statistical value of my experience is zero, and it is much more interesting to analyze the results of more rigorous analyses such as DORA (https://www.devops-research.com/research.html). In them, we can see how the following practices are associated with high-performance teams:
- Trunk Based Development (https://cloud.google.com/architecture/devops/devops-tech-trunk-based-development), where they talk about working in small batches with synchronous code reviews. (See: https://twitter.com/jezhumble/status/1415908836439773185)
- Continuous Integration (https://cloud.google.com/architecture/devops/devops-tech-continuous-integration) that implies integrate any code change in the main branch of code at least daily (https://services.google.com/fh/files/misc/state-of-devops-2015.pdf#page=20). In this context, continuous and synchronous code reviews enormously facilitate the introduction of this practice.
