The purpose of this post is to propose and highlight some guiding principles and best practices I believe developers should consider when opening a new pull request against one of our repositories. As the number of people proposing changes to our codebase continues to grow, we’ve arrived at a critical point where we need to establish some expectations and discuss some good habits so that contributors, product owners, and most importantly, customers are satisfied.
In my experience, many developers lament the pull request code review process. Rather than a collaborative experience, it becomes a confrontation between the developer and the reviewer. When I catch colleagues falling into this mindset, I like to remind them that they are the master of their own pull requests, and part of mastering your craft will always involve receiving and giving feedback. Just like becoming an experienced programmer, creating superior pull requests requires attention and practice.
Pull Request Code Review: Embrace Collaboration
Early on in my career, I would open a pull request containing dozens of commits, impacting a thousand lines of code, while providing little to no description, and expecting reviewers to read and understand my work in a relatively short time frame. The following is an example of such.
Thanks to some guidance from my peers, they suggested that while my code was valid, I wasn’t doing myself any favors by lumping my work into a single massive pull request. Instead, I should break a feature down into several easy-to-understand, easy-to-roll-out pull requests. Adopting this recommendation was a notable turning point in my journey as a developer. Ever since, my pull requests have few comments and change requests, get approved sooner, and ship to customers faster. As a result, my development process feels more collaborative and rewarding rather than confrontational and frustrating.
Smaller Pull Requests are Easier to Work With
Earlier, I mentioned that I sometimes express to my fellow developers that they are in control of their pull requests. Which may come across as a lofty ideal, but there are some real-world actions you can take to realize this premise. First and foremost, when proposing a change to the code base, as a developer you should fully understand the impact your changes will make on the system as a whole. I often refer to this as “owning the problem space”. A good developer should be able to effectively explain their design, testing methodology, and deployment characteristics as it relates to their code. Naturally, there is always 10+ different ways to solve a problem, and it’s up to you to use all available resources to cultivate context, get opinionated, and become the expert. Similar to writing good code, it’s a matter of analyzing and breaking down a feature/bugfix/task into its smallest logical deliverable components. Opening pull requests that contain only the smallest logical components minimizes the complexity and breadth of knowledge you need to convey during the pull request review cycle. In other words, smaller pull requests translates to fewer areas to comment on, raise concern, or contention with.
Another dimension to breaking large tasks into several successive smaller pull requests is that it allows for earlier feedback opportunities. Shipping more frequently provides greater opportunity to pivot and adapt your designs while reducing the time needed to go back and update existing code. To give this whole idea a catchphrase, I sometimes say “get pull request hungry”, meaning developers should strive to ship as many pull requests as they deem necessary.
Use the Pull Request Description Field
A great tool that often gets overlooked is the pull request description field. Often, I see it left empty, which was something I was guilty of in the early days of my career. There are many reasons to encourage well-written, meaningful pull request descriptions. Not only does a good description help to convey the nature of your code change, promoting communication and understanding, but it’s also a useful mechanism to identify the scope of work you’re attempting to deliver. In most repositories, I ask developers to answer the following three questions:
- What’s the purpose of the pull request?
- How was this accomplished?
- Is there any particular area(s) you want your reviews to focus on/be aware of?
This might seem onerous at first, but I find it an effective gauge when you want to ascertain whether you’ve done a good job of breaking down a body of work into their smallest logical components. If you can answer these questions in a few sentences, then you can be fairly confident that you’ve nailed it. On the other hand, if you find yourself having to write long paragraphs to explain your work, then this is a good indication that you’re trying to ship too much at once.
As an exercise I continue to this day, I like to surprise less experienced developers with review requests. If I’ve scoped the proposed change well and written a concise, meaningful pull request description, I’d expect them to have no issues with understanding and approving my code changes (provided that we’ve established the necessary context). This ensures that I stick to my principles, as well as demonstrates the level of quality I expect from my team. Here is an example of a good pull request.
Incorporate Best Practices into Pull Request Workflow
Just as crafting clean code requires practice, introspection, and refinement, so does composing smart, well-scoped, easy-to-understand, and easy-to-ship pull requests. I encourage you to consider some of the lessons and best practices I’ve learned and incorporate them into your pull request workflow. I assure you that the time you spend organizing your pull requests into frequent, incremental updates, as well as effectively communicating how and why you’re making each change is time well spent.
TL;DR: We encourage developers to produce small, well-scoped code changes supplemented by a meaningful title and description when opening a pull request. Small, incremental changes are easier to test, simpler to understand, faster to review, and safer to deploy. Everybody wins!