Creating Pull Request that “Scales”
As a software engineer, it is our job to solve problems. And mainly we solve problems by coding. These days, introducing code changes can be done by creating a pull request. Pull request (PR) lets you tell the others about code changes you’d like to introduce in a certain repository.
In this post, I am going to cover some common pitfalls that PRs usually have that make them difficult to review. After that, I will put some general ideas/guidelines on what a “Scalable” PR looks like. As a bonus, I will show you how to automate the process and measure the PR.
Problems
Multiple goals in one PR
Ideally, one PR should only have one goal. This follows the practice of the single-responsibility principle that states:
Every class in a computer program should have responsibility over a single part of that program’s functionality.
With that in mind, a PR should and always only have one “ultimate” goal. But what happens in reality? We often create a PR with a bunch of goals. This will make it hard for the reviewer because he/she needs to switch context when reviewing the PR. Also, it’s very hard to debug later after deployment if there’s an issue caused by the latest PR. There could be more than one culprit that causes the issue since the PR has many goals.
Unnecessary changes along with the main goal
Sometimes we push unnecessary changes together with the PR’s main goal like add spaces/tabs, fix typos in the variable’s name, and other changes. Those kinds of changes will only distract the focus of the reviewer from reviewing the “solution” that is proposed in the PR, and lead them to correct the “cosmetic” changes that actually can be addressed separately.
Too many unrelated commits in the single PR
This can happen when the feature branch (the branch that will be merged into the base branch) is a “shared” branch, where everyone who has write access can easily push to that feature branch. The reviewer will find it hard to see the “original” commits history and the intention of the PR as it contains another person’s commits.
Solution
So, how to deal with those situations and create a PR that “scales”?
The image above seems trivial to all of us, right? We first define what the problem is. And then we find the best possible and feasible solution to that problem. Finally, we execute the solution that is translated into lines of code. Seems quite easy, right?
It is.
But, it’s very often that we skip the solution part. We don’t literally skip it. But we instead don’t tell anyone what the solution is. We rather just show the implementation in the lines of code directly.
The ideal flow would be:
- Problem: well-defined in the development board (with Gherkin format)
- Solution: a comprehensive solution written in the technical specification (if necessary)
- Implementation: Code changes that is proposed in the pull request
But we can also change the solution part to be the description in a PR if the solution itself is not that long. This way, the author of the pull request will have confidence on the pull request regardless of the size of the PR and the type of the PR. That can also make reviewing more fun and productive for both parties. With that approach, we can make sure that the PR only and always has one clear & ultimate goal.
Yes, the next thing is: 🛠 AUTOMATE EVERYTHING 🛠
In this era where tools are widely available online (and free!), we should make use of them to ease our life. This can be in the form of many things:
- Semantic checking
This will help a lot when we do a strict check for semantic versioning in our repository. For example, it will override the title, commits, and types of the pull request so we will focus more on the solution instead of dealing with these conventions.
- Unit test
Having unit tests on the pull request is great so the reviewers don’t need to check on their local machine. It’s also for the visibility to the rest of the teams whether the PR author has implemented the test cases in the right way.
- Live rendering
It’s mainly used by a frontend-based repository in which we can see the preview URL online. This will reflect the changes that have been made via certain PR.
Apart from those tools, we also can create our own automation by having Pull Request Template. By having this, we can easily choose what kind of changes that we’ve made upfront. This is the example for the template:
And to address the unnecessary changes along with the PR’s main goal, we can, for example, implement linter checking on each repository so every commit will follow that convention. Or, if necessary we can create another small and quick-to-review PR to serve that goal.
The last but not least is to “attach” pull requests into a specific task/story in the project management board. This can be done by naming the branch with the prefix of the task/story code. That way, we can just jump from the pull request to the project board with ease so it’ll be quicker if we need to confirm the requirement. Along with this, we could also apply a strict branching strategy so, in the end, the commits in the PR are the author’s commit(s), no commits from another developer that is unrelated to the PR.
Measurement
Measurement is needed to validate those practices. After we’re done creating PR with such rules, here are the two things that considered as parameters to be measured to make sure the PR is indeed “scalable”.
Time to merge: relatively short 🕐
Generally, a good and “scalable” PR is easy to be reviewed thus can be approved instantly. But sometimes there is a big PR that needs more attention so a discussion is built to make sure that code changes are safe to be merged and released. For a starting point, a quick bug fixing PR should be approved and merged within 5 minutes. This assumes that the author and reviewer already know the context. For the feature PR, 2–3 full working days should be sufficient as the reviewer needs to go deep into implementation and there could be some back-and-forth discussions along the way.
Percentage of PR merged 🚀
Eventually, a scalable PR should always end up being approved, merged, and released to make it to production. To be more exact, a good practice is if we have ~90% of PR merged compared to the total PR raised. This number is quite high but definitely can be achieved with this “scalable” PR. After all, what’s the point of “scalable” PR if the PR itself is not approved?
When those are met, pull requests should be a “fun” medium for both author and reviewer. From the author's perspective, he/she can finish the job and also gives a positive impact on the current system by implementing the idea (as well as share the idea with other developers). The reviewer also can take knowledge from the PR be it a new lesson or a refreshment from already known one(s).
