The pull-request is a powerful tool that helps you keep code maintainability and quality. It’s mandatory in open source projects and should also be in private projects. Only two developers are needed to ensure that the project is well-taken care after, and maximize the chances of delivering bug-free releases. Be careful as it can be a double-edged sword taking time and instilling frustration if not done wisely.
How to make a good Pull Request
- A pull-request should always be made to fix one bug, refactor one part, add one new feature, etc… But only one! If you address multiple cases, please do multiple pull requests. The smaller the changes are, the easier they are to review and the fewer errors can slip into your project.
- Test your changes thoroughly, every edge case must be handled. You must also think of how your changes might bring regression to other parts of the project. The reviewer is not your Quality Assurance. Even the Quality Assurance tester is not supposed to find your bugs. You should be accountable for the code you deliver and engage your reputation.
- Read your own changes, it’s like checking your spelling when sending an email. After spending time making the changes, you may have forgotten that you wanted to refactor a part of it or left some unwanted commented code.
- If anything is left undone, it should be stated in the description and a ticket must be created in your project management system.
- When creating the pull request, try to explain in detail why and what it is for. If it makes sense, add screenshots or screen-recording to illustrate the purpose. Finally, indicate how to test the pull request by providing accounts with username and password or step by step guide to reproduce the bug or use the new feature.
Benefits of a Pull Request
- Errare humanum est. Two Errare humanum est are possible but less probable: having your code read and tested by someone else reduce significantly the risk of having your project malfunctioning. It also helps to share the sense of ownership
- Share of knowledge: As the saying says, sharing is caring. Either the reviewer is more experienced and his feedback should help the reviewed to improve, or the reviewer can learn from the code written by the expert.
- Help make sure that at least two persons in the team are aware of the code. This way, if one is unavailable the other can answer the questions.
The dark side of a Pull Request
There is no denying that it’s time-consuming. Especially for new team members or if the code is not well written or well tested.
For the first, it’s a necessary thing to help them acquire the best practice used by the team or for the team to learn from the new inputs.
For the latter, you don’t want to ship code that doesn’t fit your standard so it prevents bad code from being merged on Master and your project to be full of bug.
What a Pull Request is not
- A way to have your code tested: Finding out that the code you asked to be reviewed is not doing what it’s supposed to will leave a really bad impression on the reviewer. It will be hard for him to trust you in the future.
- A code style checker: The formatting of the code should be shared beforehand and use by all the team to avoid having the real change bring by the pull request hidden in reformatting of the file.
- A way to enforce your solution preferences: Nobody has the same way of writing code. Requesting changes should only be done to improve the project. If the solution is working well, deal with it.
- The one making the pull request should do everything possible to make the life of the reviewer easier. When writing complicated code don’t hesitate to comment on it and make sure that you test what you deliver.
- The one reviewing it shouldn’t judge the code. Make small, constructive and benevolent comments that will help the project. The goal is not to criticize the coder but elevate the project to higher ground.
In the end, what’s most important is that the project benefits from the process and don’t you forget that you will be on both sides of a pull request. And as Justin Timberlake so rightly said :
What goes around comes all the way back around
Author: Quentin Metzler (aka Q2).