Don’t Do Small Pull Requests

How asynchronous reviews and wait times harm throughput and code quality

Smaller the pull request, the better? (illustration by Jane Kim)

Introduction

In this blog post, I share my learnings in the last couple of months regarding the delivery process of our engineering team at Elli. It was during this time that I came across Dragan Stepanovic’s presentation at the Seattle Software Crafters Meetup. This post is in large part inspired by the work Dragan Stepanovic presented in his talk.

My Hypothesis

Does smaller PR size always mean higher velocity and code quality? (illustration by Jane Kim)

During our sprint retrospective, my team and I identified a core bottleneck in our delivery process. During our sprint, we worked on pull requests (PRs) that stayed open for an unusually long time, waiting for reviews, comments to be addressed, merge conflicts to be resolved, you know the deal. As a way to eliminate this bottleneck, I suggested: let’s simply slice our tasks and PRs to smaller parts. My hypothesis was that smaller PRs are easier to comprehend, thereby allowing more thorough and time-efficient reviews and, as a result, increase our team’s velocity and code quality. I was convinced this would be the solution to our bottleneck.

The Method

In order to test my hypothesis I did two things:

  1. First, I started to slice my PRs to really small parts.
  2. Second, I decided to analyse some data.

I analysed the following variables over the course of 9 months:

  • Size of the PR: the unit of measurement for PR size in this case would be the number of changes that this PR includes. Azure DevOps reports a “change count” metric for pull requests, which we will conveniently use for our data analysis.
  • PR’s Time to Completion: The total time (in business hours) taken from the point at which the PR was opened to when it was merged.
  • Reaction time (or wait time): The time taken for a reviewer to “react” with a comment or an approval to the PR.

Disclaimer: This analysis does not consider the corresponding tasks and therefore ignores the time spent before the PR was created.

The Analysis

Do we increase our velocity when we slice asynchronous pull requests to smaller parts?

To get a better understanding of the relation between PR size and time to completion, I plotted a line chart that shows the number of business hours spent until the PR merges. The graph clearly shows that smaller PRs complete faster, which we all probably assumed from personal experience. The drop-off to the far right in the graph is an interesting exception and we will get back to this in a later part of this post.

The relation between PR size and time to completion of a PR in business hours, proving the initial hypothesis that smaller PRs complete faster. (graph by Matthias Förth)

However, when I sliced my PRs really small I did not get the impression that I completed my work faster, rather it felt like the opposite. The next graph reveals that the time per change actually reaches its maximum when change count is really small (to the left) and reaches its minimum when change count gets exceptionally large (to the right).

The time required to merge a change in business hours (purple) and the ratio of PR with comments (green) depending on the PR size to illustrate the effect of the asynchronous discussion and approval process. (graph by Matthias Förth)

Ultimately our goal is not to get as many PRs closed as possible but to merge meaningful code. If we want to achieve that in the fastest way possible we should, in fact, make larger PRs, according to these numbers.

Yet, in the middle, there is an interesting minimum on the completion time per change at 5–10 changes, which suggests that slicing a 22 change-PR in half actually reduces its time for completion. Let’s look at this dip more closely, since this area is quite important (we merge 50% of our changes as PRs with a change count between 5 and 23).

What is the effect of asynchronous comments and responses?

The green line shows the percentage of PRs with one or more comments. Below 13 changes there are more PRs without comments. Above 13 changes there are more PRs with comments. This asynchronous discussion that arises as a result of comment submission leads to the increase in the time to completion. Below 13 changes, the wait time for a reviewer to react to a pull request makes up a large part of the time for a PR to complete. Above 13 changes, the initial reaction time (or the wait time taken for a reviewer to react) makes up less of the total completion time since the asynchronous discussion includes not just the initial wait for comments but also addressing them, and potentially further review loops.

How big is the impact of these reaction times actually? To gauge the size of this impact, I plotted the wait time until the first reaction (approval or comment) as a percentage of the total time taken for the PR to complete.

Comments per change as an indicator for code quality (purple) and the ratio of the time to a first reaction compared to the total completion time of the PR (pink) to show the tradeoff between the impact of wait times in the asynchronous review process and code quality.

We see that especially for smaller PRs this initial reaction time makes up the majority of the time to actually complete the PR. Yet, even though the majority of small PRs get approved without any comment from the reviewer, the ratio of comments per change is still the highest in this area. We would conclude from this that we get the most in-depth reviews on these small PRs, but for many, the code can be merged right away. So what do we make out of this now?

My Findings

In general, aim for medium-sized PRs! (illustration by Jane Kim)
  • To get the best combination of merging changes quickly but still receiving an in-depth review we should aim for medium-sized PRs.
Pass the PR-baton as soon as possible! (illustration by Jane Kim)
  • On these medium-sized PRs, the reaction time makes up a considerable portion of the total time to complete the PR. Therefore, there is quite some potential to speed up delivery by keeping any reaction time low (i.e. complete reviews, addressing comments or pipeline failures as efficiently as possible)
Do the heavy-lifting of PRs together in pairs! (illustration by Jane Kim)
  • The first chart showed a drop-off in the “time to completion” for very large PRs. This is due to the fact that many of these PRs have been completed in pairing mode. At Elli, we tend to do large refactoring’s or complex business logic implementations via pair programming. Instead of doing the review process asynchronously, where one person creates the PR and another reviews, we synchronously code and review in the pairing session. Therefore, there are no comments on the PRs and approval may be granted right away. This leads to the drop in the completion time for very large PRs, but this reduction should very well apply to medium-sized PRs as well.

Conclusion

Going back to the initial problem of long-running PRs in our retro, I would now no longer simply recommend slicing the PRs to really small pieces as a blanket solution to faster delivery times. Instead, I would recommend slicing PRs into parts such that the “heavy lifting” is done via pair-programming. The more independently manageable PRs should have a medium size to justify the wait times of asynchronous reviews.

But ultimately, there is no magic number when it comes to pull request size. However, what I did discover from my data analysis were several important guidelines to keep in mind in order to optimise the pull request workflow of our product teams. I hope these findings are useful and extend to anyone who is interested in improving delivery times within their team or organization.

Learn More

At Elli, we are always in search of data-driven ways to improve the way we work together as a team. If you are interested in finding out more about how we work, please subscribe to the Elli Medium blog and visit our company’s website at elli.eco! See you next time!

About the author

Matthias Förth is a Product Owner and former Software Engineer focused on backend development. His current interests are the streamlining of development and pushing data-driven decision making at Elli.


Don’t Do Small Pull Requests was originally published in Elli Engineering on Medium, where people are continuing the conversation by highlighting and responding to this story.

Mehr
Zurück

Weitere News

  • Social Media
    12.01.2023

    Elli vergrößert sein Netzwerk auf 400.00 Ladepunkte in Europa

    Mehr
  • Social Media
    12.01.2023

    Neues Jahr 2023, neue Ellians!

    Mehr
  • Social Media
    01.12.2022

    Ellians im Office ;-)

    Mehr
  • Social Media
    01.12.2022

    Neue Vorteile für unsere ELLIans!

    Mehr
  • Social Media
    01.12.2022

    Die gesamte MSP Business Unit vor Ort in München

    Mehr
  • Social Media
    01.12.2022

    Willkommen liebe Ellians!

    Mehr
  • Elli Einblicke
    01.12.2022

    Catch Me If You Can — Memory Leaks

    Catch Me If You Can — Memory Leaks A retrospective on a memory leak Elli engineers vs. memory leak (illus. by Jane Kim) Introduction Memory leaks are one of those things that, when they happen, can really throw you in at the deep end. Diagnosing

    Mehr
  • Elli Einblicke
    01.08.2022

    Electric Vehicle Charging for Newbies

    A quick read for all newbies to EV charging! Creating a sustainable future means changing the way we get around. Perhaps this means switching to “greener” modes of transportation like commuting by bike or public transit. It could also mean reducing

    Mehr
  • Video
    07.11.2022

    Plug & Charge bei Cupra

    Unterwegs laden- jetzt noch schneller und bequemer

    Mehr
  • Video
    07.11.2022

    Wie funktioniert Cupra Plug & Charge

    Die schnelle, einfache und sichere Art, Deinen Cupra aufzuladen.

    Mehr
  • Pressespiegel
    02.11.2022

    Volkswagen hat sich mit Miles Mobility geeinigt.

    Ab sofort integriert Miles mobility We share in sein Carsharing-Portfolio.

    Mehr
  • Pressespiegel
    25.10.2022

    Dank einer Vereinbarung zwischen Elli und Vattenfall gibt es jetzt über 24.000 neue Stationen!

    Dank einer Vereinbarung zwischen Elli und Vattenfall gibt es jetzt über 24.000 neue Stationen!

    Mehr
  • Video
    16.10.2022

    Punkte sammeln und Fahrzeug kostenlos aufladen

    Dank der Partnerschaft zwischen Elli und &Charge können Nutzer über die &Charge-App „&Charge-Kilometer“ sammeln und diese als €-Gutscheine einlösen, um sie in der Elli-App für kostenlose Ladevorgänge einzusetzen. Mehr Infos gibt es in diesem Video.

    Mehr
  • Video
    10.10.2022

    Die Elli App: Mit dem Testsieger überall laden

    Mit der Elli App an über 400.000 Ladepunkte in Europa laden!

    Mehr
  • Video
    01.10.2022

    Elli Flexpole- die flexible Schnell­lade­säule

    Die smarte Ladesäule von Elli kann fast überall aufgestellt werden. Wie genau sie funktioniert, siehst Du hier.

    Mehr
  • Pressespiegel
    27.08.2022

    Das Elektroauto als mobile Powerbank

    Die Vereinbarung zwischen Elli und The Elia Group zielt darauf ab, das E- Auto zu einer neuen Energiequelle zu machen und somit die Klimaziele schneller zu erreichen.

    Mehr
  • Elli Einblicke
    01.06.2022

    Introduction to Elli Engineering: Our Guiding Principles

    The six guiding principles for technical excellence Elli is a brand of Volkswagen Group providing energy and electric charging solutions. Software and hardware engineering are key to the business. We as engineers focus on creating and maintaining awesome

    Mehr
  • Pressespiegel
    28.01.2023

    Die Wallbox ist ab sofort bei den Volkswagen Händlern erhältlich.

    Ab jetzt sind unsere Wallboxen direkt bei den Autohäusern von Audi, Seat, Cupra, Škoda und Volkswagen erhältlich.

    Mehr