By Jesal Gadhia
Pull Requests (PRs) are a great way of submitting contributions to a project, especially when there are multiple developers working on it at the same time. If done well, they are a medium through which we can receive feedback and increase visibility for the changes we are shipping.
There are many elements to what makes for a good PR. They end up determining the quality of the end deliverable as well as telling a story of how a certain feature or change came about.
In this post, I'll cover some of the basics of what to optimize for at each stage of the PR's journey to production. My hope is that you'll be able to use this to maximize feedback quality and minimizing time to merge. So let's dive in.
Scoping
The first step in the journey to crafting a PR is to create a separate branch for your changes. At this stage, scoping your changes is key. First, think of how big of a change-set you want to introduce in a single PR. Try to address one issue or build one feature within the PR. The larger it is, the more complex it is to review and the more likely it will be delayed. Remember that reviewing PRs is taking time from someone else's day!
Once you've settled on the scope, ensure that each change is broken up into small logical commits. Having small commits is valuable because of several reasons. First, it allows the reviewer to see the progression of the changes and understand the PR in its current state. It is also very valuable if you are using git bisect to debug an issue or trying to roll back a change in the future.
Drafting
The next step in the journey is to open up a draft PR. This is not always necessary but it's useful if you want to see the full CI run with all the tests, assuming you are not running the entire test suite locally. This will give you early signals on whether your changes are interacting with the rest of the codebase in unexpected ways or not.
Take a moment to review your changes and see if everything makes sense from the perspective of the reviewer. If you are satisfied with the state of the PR, you could tag reviewers for early feedback if necessary.
If you do decide to tag reviewers, ask for specific feedback, calling out the fact that the PR is still work-in-progress. Guide your reviewers with what you want feedback on. This will ensure they don't spend time and energy giving you feedback on things that are likely to change, or things that you are already aware of.
Publishing
If your draft PR is in good shape, you are now ready to tag reviewers. When it comes to picking reviewers, pick no more than 2 - 3 primary reviewers. If you tag a large number of reviewers, chances are you might see a bystander effect.
Before tagging reviewers, ensure that:
- Tests are all green. You don’t want the reviewers to spend their valuable time pointing out the basics.
- You've clearly articulated the purpose of the PR as well as the broader context for the change. Link back to relevant Slack threads or tickets. This is important even if you think the reviewers will have the full context for the sake of posterity.
- You've done a self-review. I would recommend running through Maslow's pyramid of code review to see if your work meets those criteria. You should be the first person to critically review your work before anyone else does.
- If there are specific follow-ups that you have in mind, describe those as well so the reviewers know what to expect down the line.
- You have provided related artifacts to the PR in the form of screenshots, videos or logs. This will help your reviewers better understand the impact of your changes.
Feedback
When you receive feedback from the reviewers keep in mind:
- Tone can easily get lost in async communication. So assume the best intent and don't take the feedback personally. Offer clarification and context for your decisions.
- If something isn't clear, ask clarifying questions, don't try to read the tea leaves. If things are still not clear in async conversation, request some time to chat on a call. Once you’ve had a chance to discuss, circle back and summarize the discussed points in the PR to preserve context.
- Whether you decide to take the feedback or not, be sure to acknowledge the feedback you've received. This demonstrates that you value the reviewers' time and effort.
- Explicitly re-request review once you are done addressing the feedback. Don't assume the reviewer will know when you are done. Bonus points if you follow-up with a comment pointing out the specific commit that addressed their feedback.
Releasing
Once you have gone through the feedback cycles and gotten the approval of reviewers remember to come back and merge your PR. The longer you wait, the greater the chances of encountering merge conflicts or regressions. So be sure to merge as quickly as possible.
If you are releasing a PR that will perform a large data operation or change something in the critical path, ensure that you are available to follow that PR through the release process and mitigate any errors or hiccups along the way.
If all goes well and your changes are in production, remember to follow-up with the stakeholders and update them on the progress.
About the Author
Jesal is passionate about applying technology towards meaningful, positive impact. He started writing code for fun in middle school. Since then, he has amassed over a decade of experience in the tech industry ranging from being a co-founder to an engineer. In his free time, he enjoys playing with his son, tinkering on side-projects, and traveling.
Join the conversation