Recently my colleague Daniel wrote an article about Our Code Review Guidelines. But there is another component to this equation: the quality of the Pull Request (PR) that originated it.
Several years ago, when I was a newbie Rails developer, I submitted my very first pull request to an open source project. It had only a very basic description about what it was doing, with no further details about why or how. This was my PR in its entirety:
Needless to say, it was not received with gratitude and immediately merged like I had hoped. Instead, I learned a valuable lesson in what constitutes a good pull request. This has stayed with me and shapes the way I write PRs both now and in the future.
When writing a pull request, I always make sure to include the following key components:
- State the problem
- Describe the fix
- Explain how that fix will make the code better
Why is a good PR description so important?
Firstly, it is the best place for you to communicate your changes with your team, particularly a remote team. Although your code shows what was changed, it doesn’t often show why. A comprehensively-written PR is an opportunity to explain why you did things the way you did.
Because a PR remains in the project history forever, it provides a reference to decisions made and the reasons behind them. This is helpful not only to the reviewer in the immediate moment, but also to your future self. :-)
Finally, it gives context to the person reviewing it. Sometimes the reviewer is not part of the project, and a well-written PR can provide them with the additional information they need in order to give you their best feedback.
What should I put in my description?
- What the PR solves (reiterate what is the issue or story, and link to the issue wherever possible)
- A high-level description of how you achieved the goal
- Mention whether this PR is completed, or a work in progress
- Any considerations and assumptions you made in writing your code
- A screenshot, animated gif, or terminal output to illustrate what changes have been made
Here is an example of a pull request that I submitted recently. I might be somewhat biased :-) but I believe this is a great example of what constitutes a good pull request:
Let’s break it down…
At the very top, it specifies which issue (complete with link) this pull request solves. This addresses point number one above:
Immediately below that is a quick description of what was introduced, and how it solves the issue. While the reciever can see the changes that were made by viewing the diff, they will not necessarily know why those changes were made:
Following that is more detail on the high-level description:
Then it notes that this particular PR is not complete. It states that it is intended to introduce the solution and requests feedback from the maintainers:
Next we see a list of considerations explaining why things have been implemented the way they have. Including this allows the reviewer to suggest alternative ways of achieving the same outcome, if they so wish:
Finally, there is a screenshot of the output as it will look once this PR is merged. It gives the reviewer confidence that it does what it says it will do without them having to set up the project just to review your PR. This is a key point: if your reviewer is not on your project (as is sometimes the case here at reinteractive), they can spend extra time cloning and setting up your project just for the sake of providing PR feedback:
When providing a screenshot, be sure to consider the following:
- how the changes will look (if implementing a new feature)
- how it looked before, and how it looks now (if changing an existing feature)
- You can also include any terminal output that is applicable
- Screenshots (before and after)
- Animated gifs/videos
Do I need to have all these sections in my PR?
Not necessarily; you need to consider what is required on a case by case basis. But you must always include enough information so that someone who doesn’t know anything about the project can understand why the changes were made.
Your PR should include enough information so that the reviewer doesn’t need to check out your code to test it (although this is still a good thing for them to do).
And finally, having better and more detailed descriptions will make the reviewer happier and consequently you will have better reviews.
