I don't know about you but I love the feeling of getting to hit the merge button on my code and sending it off to production. That's our ultimate goal as software engineers - to get our code out there into the world. However, unless you're living life on the edge, there's a big hurdle to overcome before getting to push that merge button - getting approvals on your pull request. Let's talk about how to make the best of your PRs that way your reviewers know exactly what they're looking at and you get to hit that merge button faster.
Pull requests are telling the story of your changes. They are a conversation between you and your reviewers and as the author of the story you want to make sure the process of reviewing your PR is as easy as possible. It is vital that our PRs give everything to our reviewers that they may need. They should concisely describe our motives and thoughts behind our changes all the while preemptively answering any questions our reviewers may have.
Outlining Your PR
Just like a good story, our pull request should begin with an outline of its chapters. In our case, our chapters are our commits. We've all seen commits that provide absolutely no insight into what has gone on but each commit message should show a progression in your pull request story.
The subject line of your commit should give an overview of the changes made while the body should provide additional context such as why the changes were made, any possible future implications, and a reference to the ticket or issue number.
Using conventional commits can help provide even more at-a-glance insight.
Organizing Your PR
Having explicit commit messages is important but in order for your story to be understood those commits need to be organized in a way that makes sense. You want to make sure your reviewers can easily follow the story you're telling about your changes.
But what if you have to implement a fix or refactor something later on and you've already made commits? That's totally ok! As developers, we have the power to change history with rebasing. If you aren't familiar with rebasing it allows us to rework our commits. You can change the order, rewrite your commit messages, or even squash two or more commits together. Here's a quick TLDR on the difference between rebasing and merging.
Paying Attention to the Size of Your PR
If our commits are the chapters to our pull request then our actual code implementations and changes are the story itself. It's important that we pay attention to the size of our story. There's a computer programming principle called the Single-Responsibility Principle. It states...
Every module, class or function in computer programming should have responsibility over a single part of that program's functionality, and it should encapsulate that part. All of that module, class or function's services should be narrowly aligned with that responsibility.
To summarize, a single module, class or function should only focus on one thing. That same concept applies to pull requests. You may think that a PR that touches a lot of files would receive more comments than a smaller PR but a study revealed that developers should review no more than 200-400 lines of code at a time. Beyond 400 lines of code, the ability to find defects diminishes.
So by breaking a large pull request into several smaller ones you're actually increasing the chance that you'll receive feedback and the potential for your reviewers to catch a bug they may have missed if the PR was larger.
Your PR's Introduction
We’ve covered the chapters to our PR story and the story itself but what’s a good story without an introduction? That’s where our title and description come in.
A pull request title is the first bit of information you're providing to your reviewers for what they are about to look at. It needs to give a concise overview of what is happening in the PR. The description is where you can provide more detail for your reviewers or anyone looking at your PR in the future.
Remember to never assume the reader's prior knowledge of the area of the codebase you're working in. It's your job as the author to provide the context to them. You can do this through including information on the "what", the "why", and the "how" of your changes.
The what should provide explicit details on the changes in your pull request. Remember how our commits are the outline of our story? This is where they come into play. Use your commit messages as a baseline for explaining your changes to your reviewers. Expand on what you wrote previously with a bit more detail. This section of your description should also include any lingering TODOs linked to their follow-up ticket.
The why should explain the reasoning behind your changes including any architectural decisions you made and any possible implications from those decisions. This can include user stories for why this specific feature was added, thoughts behind refactoring you did, or even explaining your thought process.
Lastly is the how. How are your reviewers supposed to test your code? Go through the steps to reproduce your changes in a demo environment. You want to be as explicit as possible - provide direct links for the route that needs to be tested and any feature flags or permissions they may need. You also want to make sure the exact scenarios you want tested are laid out. For example, certain steps your reviewers may need to take to show an error state.
Become Your Own Reviewer
Before adding any reviewers to a pull request I become my own reviewer. I look through each of my commits to make sure they're linear and self-explanatory and take a look at the code itself too. While doing this I often annotate my own PR commenting on specific lines of code that may leave your reviewers with questions. You can use this to explain why you chose to do something a particular way like if the direction to go was a little outside of the norm or you can highlight a line of code to get more opinions on it.
Continuing the Conversation
Now that you’ve put together the story of your pull request, added your reviewers, and officially opened your PR you would think that things end there, however, the conversation around your changes are only beginning. The entire point of a pull request is to get eyes on your code so others can catch bugs or provide feedback. An essential part of the PR process is responding to that feedback.
Pull requests are a story and you should be having conversations around that story with your reviewers. Whether you only get one or two comments or ten, you’ll want to make sure to reply to every single one. Pull requests should not be merged until every comment has been addressed whether that’s implementing a suggestion or not. If you do apply a suggestion from a reviewer be sure to acknowledge that.
Not all comments will be within the scope of your pull request, however. Ultimately, it’s your decision what is and is not within scope but if something is outside of the scope then you need to create a follow up ticket. The key to a great pull request is being completely transparent. Explain to your reviewer why their suggestion is out of scope and include the link to the follow up ticket where it will be addressed.
After all comments are addressed it’s finally time - you get to push the merge button! You’ve accomplished your goal of merging your code to production and that’s another ticket marked off your to do list. Now it’s just time to start the entire process again with a new one.
What are your tips for making a perfecting pull request? Comment below!
Be sure to follow me on Twitter or Threads for lots of posts about tech, and if I'm being honest, lots of posts about dogs too.
Latest comments (13)
Great perspective. Thanks.
Thank you for this great guide. I can only agree with everything you said.
Awesome stuff.
Another angle you might approach on the same theme is the technical (this covered the human side very well, and I wish it were obvious, but hey, I'm glad, given it clearly isn't to many, it's got some coverage). The technical side that still befuddles me at times is characterised by the symptom that when I issue a PR on github I am surprised by a sudden explosion in commits! Go figure, I don't understand why, and I'm keen to learn how to avoid it.
The workflow that lands one there is as follows:
The technical workflow for managing upstream, origin and local and a local branch with a change that wants to be a PR, given upstream has moved on since branching ... is what interest me. How to get a clean PR with only the commits I intend to deliver and submit for review is the question I have not solved.
Great post!!
Thanks for sharing
great job !
Wild how much effort and politics go into a pull request in a company. Good advice.
Very good!
Thanks for sharing Kara!
I believe that the what and why of the PR should be well described in the ticket of the tasks (Jira maybe). Its ID should be present on the name of the branch and its link in the PR comment. The how should probably be clear from the code itself(good naming, small functions, encapsulation, clear API...) documentation and the tests.
All that should minimize the need to explain that in the PR and it is probably more future proof.
Some great tips in here. Big fan of being your own reveiwer. I always catch something when I review my own PR. It helps avoid wasting other peoples time too