Before opening a PR
I advocate for using before-commit hooks in Git. These hooks enable you to run custom scripts before commiting changes. There a quite a number of quality checks, if you ask me (and you kind-of do, since you are reading this! 😉), and almost all of them should be run before comitting your changes. I’ll mention the checks later on, as well.
Baseline measurements of a PR
One of the most important things to remember when doing PRs:
Don’t let them get too big. A size I found to work well was around 400 lines changed. While this is no hard limit, going over it tends to incorporate too many changes at once. This makes the review harder for the reviewers.
Another thing not to lose track of, is the number of days a PR is open. You should review PRs as soon, as they are assigned to you, or when you are being mentioned in their description (or in comments). It is your duty not to let your team wait for too long on your feedback/review. If there are no major errors in the changes, a 400 lines feature should be reviewed and merged within 1–2 business days.
Who do you assign a PR to? Who should review?
This depends very much on your team structure and how you are organized. If it is possible, it would be great to get two people to review your changes. A healthy mix could be one person from your immediate team, and another one from a different team. This can have the benefit of a different perspective. The one from the outside-team probably doesn’t know too much about your feature and the specifics of your code. This forces you to be extra detailled when writing your description and documentation and they might not make some assumptions that you and your team take for granted. This situation needs to be handled carefully, you don’t want to get derailed by fundamental questions. But it could also prove to be beneficial to your feature, ymmv. It also increases knowledge across team boundaries, which I consider to be good practice.
If cross-team doesn’t work for you, try to get a more experienced (or same level as you) and a less experienced person from your team. While both reviewers will help you improve your result, the less experienced might get a chance to learn from reviewing your code.
In any case, @mention your reviewers in the description and (if possible in your software) assign the PR to the one you want to be responsible for merging.
What should you write in the description of a PR?
The first thing that you should write is a good title. This is the first thing that reviewers see, and it usually also goes into the emails they get about the PR. So make sure they understand what it is about. This is also a good place to let the reviewers know if you only want a review, without the goal of merging the PR. Perhaps you have an early draft of your feature, and want guidance on the general direction. You can do that by prefixing your title with something like
WIP: Integrate Stripe as payment provider
This WIP (work in progress) makes your intentions clear. If you are using GitHub, common practice is to use labels for that. So you could have a “Do not merge” label, or again a “WIP” label. I tend to always write it into the title as well, so people can’t miss it.
The next point to take care of is the description. This is where you should spent most of your time, when creating a PR. Consider writing about the following points:
- What is changed?
- Why was the change necessary?
- How does the project / part of the code / microservice look after making your changes?
- Are there any side-effects of your change?
- Do you want to add any commentary about your changes? Perhaps link to Stack Overflow answers that helped you, or blog articles and books you referenced during development?
- Is there anything unclear with your changes? Perhaps you have a road-block that you need help with? Or you are uncertain how your changes interfere with another component? Ask about that. This is the place to do that.
If it makes sense, you should add screenshots of the before/after situation. If your changes contain animations or important visual effects, provide a gif or a short screencast of your changes.
Finally, make clear what you want the reviewers to look for, test or what they should comment on. If they follow this guide, they will look for more than that, but it is always appreciated to get a little guidance from the author of a PR.
Don’t forget to link/reference the issue/ticket in your management software that this feature/PR belongs to.