5 things every pull request should have. Little thoughts from my experience.
TL;DR: Here is a link to my PR template following the best practices described in this article
Intro: PRs with many parents
The bigger the company the harder it is to achieve consensus over pull request creation, redaction, and formatting. From the number of files to the number of lines to the commit messages, opinions come from multiple fronts and PRs will have many parents, each of them with their own likes.
From my point of view, creating a pull request can be compared to writing a chapter in a book, the smoother you do it, the more it will engage the reader. Of course, there are multiple opinions on this matter. From quite short, even non-explicatory pull requests, to overwhelming rivers of words that tend to draw the reviewer.
This article aims to show my own thoughts on Pull request creation, which I have gathered from my own experiences and talking with fellow colleagues.
1. Be granular
Try to not overcomplicate things by throwing everything you have into the box. Be a surgeon with your code and separate single responsibility files or files within the same topic. Reviewers will gladly appreciate these efforts and the life cycle of your PR(Pull Request)will be shorter.
“The slower you move while coding, the faster you’ll get things done”
There is an engineer I have been following for quite some time that says: “The slower you move while coding, the faster you’ll get things done” , meaning that rushing the code or its merge into master is never a good thing.
Bad code or not reviewed-enough code might end up breaking the production environment which will be translated into a loss of money.
Think of it as a big pie, if you eat it piece by piece, you will enjoy it and be happy with it. However, if you eat the whole lot at once you will feel bloated and possibly will hate it afterward. The same applies to lines of code and files, the easier it is for a reviewer to digest, the faster he will review it.
2. Be graphic when needed
I have seen so many pull requests get stuck for weeks in code review because reviewers could not figure out how it will look and did not have the time to pull it locally and run it.
If your pull request involves a visual change or could use any visual support as support, include it. Whether it is a change on the UI, a UML explaining a system, or a decision tree for an algorithm, include it.
Reviewers will kindly appreciate it and reviewing your PR would be easier. In the end, a picture speaks a thousand words.
3. Explain yourself
Many PRs that I or some of the people I have talked to are way too abstract or obscure to grasp everything at first, and it might take a long time to figure everything out. In those cases, I would highly appreciate some hints on how the code works.
Imagine for a second you are presented a recipe, a picture showing how the final result will look like and the ingredients.
Cool. Now to the important part, How do I cook it ?! You start stressing and squeeze your brains out to figure the recipe. Maddening, right? Well, it is the same with a PR.
Think of it as if somebody that just started working today, was taking a look at it. They will feel lost in the code, as you would when cooking a pie without the steps.
However, if we add suitable comments on key parts of our code (a method from a third party library, something that comes from a very complex recursive method,..), it will help them understand it better, faster, and easier.
The same as if we were given each step when making a carrot cake, it will taste way better if we knew the correct steps.
In order to support this, you can add references to other lines within the PR or event external sources that can help give more sense to your code
4. Always ask yourself if tests are needed
I know it may sound a little dumb to add this to the list but, sometimes, we are so used to tests that they slip from our minds and we end up with untested code.
It might not mean a lot at the moment, but think forward in time, 3 maybe 6 months into the future, somebody changes a line that returns an integer instead of a string, and the code gets deployed, breaks the production environment and a rollback is needed because users cannot validate their purchases.
This could have been avoided just by adding one test that covered the code. Think of the repository as a team, we have to make work easier for everyone, not harder.
5. Be clear and polite
Clarify each comment on the PR, even sync with the reviewer if needed, to explain the code’s behavior.
Many times, two brains are better than one when detecting possible failures
Also, do not take any comment as a personal offense. Nobody wants to sabotage your code. Online comments might sometimes sound rude as we do not have the person in front of us, but it is always good etiquette to answer comments as good as we can.
If you get to the end of this article, first of all, thank you for reading!! Second of all, here is a link to a repository containing my template for PRs following the points I have explained here, feel free to fork it or create any PR to update it.
And that is all for today folks!! Thanks a lot for reading the article and feel free to drop your opinions or questions!
See you around 💃
Pablo.
