As a developer in a professional environment, you’ve most likely have had to perform code reviews with your colleagues.
Code Reviews and Pull Requests (PR) are a fantastic learning tool and can be a point of discussion for all developers. Having other sets of eyes inspecting your code will assist with reducing errors and streamline the process of software development drastically.
The aim of this post is to give you some tips and guidelines to set the stage for good PR etiquette. Take these tips with a grain of salt; all software development teams work differently, and one set of guidelines and rules might not be a “one size fits all”. Take what you can from this and see if there are any areas you can improve on or incorporate into your PR system. Feel free to share any tips and tricks you use as well!
Just as a note, I will continue to update and add more to these tips and guidelines, since me and my team are learning more and improving our PR etiquette as we go along. I’ll be sure to share them with you here, if I find any that are interesting.
Tips and Guidelines:
- Before sending your PR out for review, have a look over it yourself. Do a self-review of your own code. You will likely catch some minor styling/formatting issues, or possibly even major issues right away.
- Always include a brief description of your changes, to give some background information for reviewers, so they will understand the reason for any changes.
- If there are any related tickets or pull requests, be sure to reference them accordingly.
- Try your best to keep PR’s small, or split them up if possible. Don’t bombard your team members with a monolithic review. Sometimes this may start with breaking down the actual ticket or task itself. It is much easier to do it before the fact, rather than after.
- Add in your own comments to the PR to help guide the reviewers. Highlight any areas that they should focus on, and clarify any changes that may cause confusion. Sometimes, there may be follow-up tickets that will implement another feature, or missing logic in your current branch.
- Ensure there are no merge conflicts, and your branch is up to date with the target branch.
- Keep everyone in the loop; if you discuss things offline with another reviewer, be sure to log it as a comment in your PR. Something as simple as “As discussed with X we are not going to do Y”.
- If your PR is just an abomination to go through, you may want to meet up with your reviewers beforehand and go over it in detail. (Especially for major architecture reworks).
- Screenshots are especially useful if there are any UI changes.
- Give ample time for reviewers to go over your code.
If you have anything else to add, feel free to share in the comments!