How to ace your pull requests
At Silverstag Consulting, we are big believers in incremental improvements producing monumental results. As such, we put a lot of time and effort into improving and finely tuning our day-to-day activities. Our pull request (PR) process is one of these activities.
What is a pull request?
A pull request (also known as a merge request), in its simplest form, is a request for other team members to review the code that you have written. When we write code, we mostly write it in isolation — i.e we write it on our own machines, and we are the only people who see it (initially). We create PRs in order to receive feedback on solutions and coding practices from other members of the team, ensuring all newly written code adheres to company standards. PRs also allow every member of the team the opportunity to weigh in on the proposed solution, opening the door for fresh perspectives and collaboration.
Why are pull requests valuable?
A PR serves as the final checkpoint before any new code makes its way into the system, it’s the final security measure where the whole team takes responsibility for the new code and works together to ensure that our base branch retains its high level of test coverage, maintainability, and quality. If a PR were a fictional character, it would be Heimdall — the norse deity responsible for protecting the Bifrost (in our case, our base branch). If the code being reviewed is approved by enough reviewers, then it has received the team’s stamp of approval and ultimately makes its way into the code base; if it is rejected, it remains isolated until the requested changes have been made.
A successful PR process allows us to constantly keep a tidy house, cleaning up after ourselves as we go. Without this process, we could one day wake up to a very big mess which would require far more effort to clean up that if we simply addressed the mess when it was created.
If we approach PRs professionally (be it as a reviewee or as a reviewer) we can ensure that the majority of code smells, domain blunders and inefficiencies are caught before they become a larger problem. It is important to remember that any code that is wreaking havoc in your environment would have had to go through a review process at some point — and at that time, it was considered worthy.
PRs add value in the following ways:
- Allows team members to weigh in on the manner in which the problem was solved — there could be an easier way or the developer could have missed something glaringly obvious or something far more obscure.
- Allows team members to provide guidance on code style and implementation. This promotes knowledge sharing amongst the team, and will ultimately strengthen the team as a whole.
- Gives the entire team an overview of the feature that was developed or the bug that was squashed. This improves domain knowledge for everyone involved.
- Ensures that rogue code (if reviewed properly) does not leak into the existing code base.
So, knowing what we know, how can we make PRs even better?
How to improve your Pull Requests
At Silverstag Consulting, we have identified some common mistakes that developers make when creating pull requests. By addressing these, we have found that our rejection rate, as well as the number of change requests per pull request, have been reduced dramatically.
Review your own code first
Before adding reviewers to your PR, take the time to review the code yourself. You will be amazed how easy it is to spot mistakes when scrolling through your own code. By doing this, you not only catch the most obvious mistakes before another developer can, but you also show respect for your reviewers’ time — they shouldn’t be catching issues that you can spot yourself..
Reviewing pull requests is an active process, and by ensuring that those you request reviews from are focused on real problems and not obvious ones, you ensure that they will give your PR their full and honest attention.
Keep commits atomic and descriptive
A pull request is made up of one or more commits, keeping your commits atomic makes it much easier to understand what you were working on at any given point in time and which part of a feature or bug fix the specific commit relates to.
Targeting a small commit with a commit message of “Added submit logic to submit button” is much easier than trying to dissect a large commit with a commit message of “Completed feature” — this holds true outside of the PR process as well, it’s a lot easier to debug and understand code when the associated commit message is descriptive and to the point. If this is something you find difficult, consider using a commit tool like Husky which is a great way of ensuring commit messages are descriptive and standardised across a repository.
Remember that your job is to make your reviewers’ lives as easy as possible, because the easier it is for them to understand your thinking, the more likely it is that your pull request will be merged without complaint.
Keep reviews feature or bug focused
The code that you put up for review should always be the minimum amount of code required for a single bug fix or feature. This ensures that the review itself covers the smallest amount of code possible. There is nothing worse than having to review a massive pull request — not only does it take up unnecessary time due to its size, but it also makes any requested changes just as difficult to navigate. If the pull request is large enough to warrant, say, 20 changes, the re-review of those 20 requested changes will take just as much time, thereby making the entire process one long, painstakingly difficult task.
Huge PRs lead to sloppy reviews as reviewers rush to try get through all the changes (remember, your reviewers have other priorities as well) and might not give the changes the attention they require.
If your feature or bug fix is so large that you are unable to keep the amount of code at a minimum, sticking to the previous suggestion (keep commits atomic and descriptive) will at least make your PR a little easier to navigate.
If you are consistently submitting PRs that span multiple features or bug fixes, a more effective approach would be to discuss the negatives with your project management team (provided that they are not making an effort to limit the size of each story). Stories should always be small enough to tackle in isolation, and having to work on multiple features at once will ultimately result in more bugs making their way into production — not only is your focus scattered, but reviews become harder to manage and mistakes are easier to make.
PRs are just one of the pieces of the puzzle at Silverstag and we are constantly looking for ways to improve our processes. If you have the same mindset and are passionate about building high quality, industry shaping products, contact us at hello@silverstag.io!
