Nowadays it’s a common collaboration practice in software development to open a Pull Request. It helps your other team members be on top of committed changes and develop the best code you can, while also decreasing the number of bugs that the software contains.
There are good and bad practices for delivering new updates. As engineering managers, we should look for opportunities to improve our team’s process and increase our output quality in the search for a decreased lead time.
This article talks about why Pull Request size matters, what the pros and cons of smaller Pull Requests are, and how to go work with them. We will also show you how Athenian can help you monitor your team culture in terms of Pull Request size.
It seems logical that it’s easier for humans to work on smaller tasks. So why do we often end up with large Pull Requests anyway?
When us developers work on a new task, the first thing we do is try to make sense of its scope. In the planning phase, we want to know:
We are also all tempted to sometimes complete everything in one shot, not stopping for feedback at any point. By breaking the task into smaller parts, we find natural stops where we can take a moment to breathe and pull others into a collaborative process.
There are times when we can’t see the full scope in the planning stage, but we have to get hands on. Sometimes it’s as simple as doing some research, but other times we need to learn a new tech or code base from scratch.
If that’s the case, we start prototyping to figure out the full picture along the way. The problem with prototyping is that it’s difficult to structure the work into smaller pieces if you don’t know where you’re going just yet.
We have all been tempted to just remove a line of code because it was somewhere we didn’t like. Or saw a variable name we wanted to change.
To have effective code reviews, you need to avoid including non-related changes inside a single Pull Request. If something is unrelated to the current task and needs immediate action, it should be moved to another branch where the change is made (with an additional Pull Request!). It’s a small action, but has many benefits discussed in the following sections.
Have you ever been assigned to review a Pull Request with no idea where to start? Those are the types we should be afraid of! They usually have two outcomes:
A delayed review
Reviewers know that code review is part of the job, and so it should be done diligently. But to do it properly on a large Pull Request distracts time from other tasks, so they would rather postpone it.
A decrease in review quality
Reviewing work that is not ours can be difficult, especially in programming where different people have different styles and frames of work. We can’t just read through thousands of lines of code and immediately understand what’s going on. Consequently, it’s more difficult to spot problems, suggest improvements, and understand which parts of the code were affected. Reviewers might think they grasp the change, but do not truly understand its intention.
However, problems are not just related to code reviews. There are additional negative effects:
A larger potential for errors
A large piece of code has many side effects and moving parts. This makes it a huge challenge to be properly tested, even for the person who just wrote it. Similarly, it’s hard to imagine all the corner cases and cover them with automated tests.
The collaboration quality decreases
When we invest a lot of time into a large piece of work, we naturally become personally attached to it. As a result, we become resistant to accepting suggestions or comments that feel like they detract from the initial effort.
If tasks are split into smaller batches, problems will be spotted earlier on. Spending weeks on something which might or might not work is time-consuming, and therefore expensive in both money and opportunity cost.
DORA’s 2018 State of DevOps research discovered that many High-Performance Organisations keep their Cycle Time below 24 hours. This means that they deliver updates on a daily basis.
This is in part also because of a company’s culture and work processes. These feed off of each other, and have huge benefits for output quality:
Find below some quotes of High-Performance engineering teams talking about how smaller Pull Requests helped them:
Remember: this is all about teamwork. Some things might make you a little slower, especially until you get into the right frame of mind, but it will make the whole team faster in the long run, and will also increase the chances of bugs being caught during code review. A final plus is that knowledge-sharing will also be better, since there is less to learn on each PR, and team members can ask more questions without being afraid of turning the review process into an endless discussion. - Zalando
We have over 13,500 customers in over 90 countries, serve millions of requests, and deploy 300 times a day. As you grow to that scale, one of the biggest challenges of building software is how you maintain the nimbleness you have as a younger company. When I talk to people about how product development works here at HubSpot, a common theme emerges: we keep things small. - Hubspot
Developers should review no more than 200 to 400 lines of code (LOC) at a time. The brain can only effectively process so much information at a time; beyond 400 LOC, the ability to find defects diminishes. In practice, a review of 200-400 LOC over 60 to 90 minutes should yield 70-90% defect discovery. - Cisco
By now you should be convinced that smaller Pull Requests are an important topic. But to achieve this, you need the whole team on board.
Each and every Pull Request should focus on one thing (and one thing only!). Make sure that you change only one artifact at the same time.
Example: If you’re building end-to-end implementation of a new form, split it into UI, data flow or domain changes, and database management. Start with what is most difficult, to quickly resolve all unknowns and make each part as independent and self-contained as possible. This will prevent blocks by code review or previous merges, and enable you to continue working on the sequential parts.
Be aware of other changes that pop into your work during any particular task. If you need to fix a bug or spot a needed change, don’t let it be part of the same Pull Request.
Have clear output expectations
Everyone should know that they can simply ask their Engineering Manager or Product Owner if they don’t have a clear understanding of the output of a task.
To make life easier, a new task should be started by thinking about the most challenging parts. These can then be tackled first. When you remove all the unknowns, you can properly estimate a feature delivery. At the same time, work can be structured into meaningful parts. If you’d like to know more about this specific topic, we recommend Shape Up by the Basecamp team.
Think about the reviewer
Make sure everyone will do code reviews. This is the only way that people will really understand how hard it is to do a good review over 1000 lines of code, or a Pull Request with non-related changes. Self-reviews will also make developers reflect on the Pull Request structure, so they can add additional context if needed or decouple work into more Pull Request (hint: use git cherry-pick).
Another way to consider is if thinking about how you’d represent a Pull Request in one sentence. If it’s impossible/too complicated/too long, then it might be time to decouple. There should also be a succinct context in the title of the Pull Request, so reviewers have an immediate context. This is especially crucial when you granulate work, as some changes can be abstract and need additional explanation. Describe how you structured your work, and attach a link to the task ticket.
Introduce feature flags
Decoupling into smaller parts can leave you with quite some unfinished work as part of production. Feature flags are useful tools for hiding unfinished work from customers and stakeholders. This also gives the ability to test code in production, or deliver it gradually. This minimizes the risk that all customers are affected by a recently deployed issue.
There is no doubt that decoupling work brings some overhead that you’ll have to get used to. Git gives you the ability to do much more than creating a new branch and pushing code to the cloud.
git stash - Using git stash you are able to save your current work for later, without doing a WIP commit. You can switch to a new branch, refactor or bug fix, and then return to the previous work. This won’t leave any additional commits in the project history, and work will be decoupled in a reasonable way.
git rebase - When there is a lot of work in the main branch, you are quite often behind the latest commit. On the other hand, merging will leave you with many merge commits, which leave a mess in your git history. So instead of merging, you can use git rebase for the scenario when you just want to get the latest work from the public branch into your feature branch. Be aware that git rebase can cause you some trouble if you don’t use it cautiously. Read through the Atlassian article comparing git rebase and git merge.
git cherry-pick - Individual commits can easily be turned into Pull Requests with git cherry-pick. You can simply list commits that you would like to submit as a new Pull Request. To do that, switch to a new branch and bring listed commits over with git cherry-pick. You can also combine it with git stash, which helps you deal with uncommitted code as described above.
Using Athenian you can quickly check each team’s Lead Time, as well as time spent in each part of the Software Delivery Pipeline. Moreover, you can go dive deeper into each segment, check insights and spot Pull Requests that are stuck in the process.
In terms of Pull Request size, you can see the average size of Pull Requests compared to lines of code changed and files affected. You can also check how this influences the code review waiting time.