Pull Request Principles in Mamikos
Code review is a very important part of the software development cycle. On Github and some hosting repositories, Pull Requests (called Merge Requests in Gitlab) or commonly abbreviated as PR, are used to review code on a branch before it can reach the master or target branch.
First, let’s admit that reviewing pull requests is hard. Imagine someone writes a ton of codes and You, as the reviewer is expected to read, evaluate, and respond to the implementations of new features or systems. There is a lot of stuff a reviewer needs to do, especially in a large pull request.
There’s a quick video from Joma Tech about the ineffective code review activity that might happen if we don’t make the code review process easier:
So, how can we make our pull requests process easier for the reviewers? In Mamikos, there are two principles we always follow every time we (as a developer) want to create a Pull Request:
1st Principle: Make a smaller Pull Request
Making smaller Pull Requests can speed up the review time and quickly get feedback. Because they are small, it’s easier for the reviewers to understand the context and reason with the logic. If the functionality you are working on is too big or complex, it could be helpful to split it into smaller Pull Requests.
I don’t say Feature breakdown is an easy task. Feature breakdown is understanding a big feature and breaking it into small pieces that make sense and can be merged into the codebase piece by piece without breaking anything.
Feature breakdown is an art. The more you do it, the easier it gets.
2nd Principle: Write a better description
In Mamikos, we always believe that Pull Requests are just not for reviewing a code, but it is also a record of the codes about WHAT changes were being made, WHY it was made, and HOW did the related developer solve the problem.
It will become a permanent part of the Version Control history, and will possibly be read by hundreds of people other than the reviewers over the years. Future developers will be able to search for the Pull Requests based on their descriptions. Someone in the future might be looking for your change because of a faint memory of its relevance. If all the important information is in the code and not the description, it’s going to be a lot harder for them to locate the PR.
Why Should I Care about Description?
Think of a PR like your own product. Every time you create a PR, someone called a “reviewer” is your customer. You want the customer to “buy” your Pull Request by approving it so your feature can be deployed quickly. Since most of us have likely been reviewers already, simply put yourself in the shoes of the reviewers and ask, “How could this be easier for me?”
If you’re still not sure why descriptions are important, here are the reasons why we should write them:
1. They Capture Context
Of course we always be able to go back and look at the result of each pull requests in the past. But, what you won’t find if there is no clear description from the pull request’s source code is, “why those changes were made?”, “what problem those code was solved?”, “How this code worked?”, Or even an extreme question, “Why does this code exist on this planet?”.
A good Pull Request description contains the answer.
2. Descriptions lead to better solutions
A good description can lead to a great discussion on a Pull Request. It’s not possible to always come up with the best available solution on your own. In most cases, there will always be someone who will know an area of source code better than you. The reviewer has an opportunity to suggest alternative and possibly better solutions to what was suggested in a PR.
Without a description that draws the reviewer’s attention to the approach, you’re significantly reducing the chances that a better solution will emerge through code reviews.
3. Descriptions make reviewing easier
A good PR description prepares the reviewer for what they’ll find during the code review. They can tell the reviewer where to focus their attention by highlighting areas that you’re not sure about as well as areas that you’re proud of. This makes it easier to process PR during code reviews and quickly provides meaningful feedback.
4. Descriptions make you invaluable
Most of our days as developers are spent finding solutions to problems. If you spent a week working on a problem, spending an hour to write a detailed PR description will make your solution look that much better because it’ll allow your teammates to understand why this problem was complicated and required a week of work.
—
The Anatomy of a better Pull Request description
“Ok, now I understand why it’s a matter. But how can I write a better description? Are there any rules that I can follow?”
1. Title
Keep in mind that the PR title should be informative enough that the reviewers don’t have to read your PR or its whole description just to get a general idea of what your PR actually did.
For example: “Change the auth system mechanism”, “Update homepage slide banner”, “Add encryption to the tracking process”.
2. Informative Body Description
The description should be informative. It might include:
✓ A brief description of what problem that’s being solved.
✓ Why this is the best approach.
✓ If there are any shortcomings to the approach, they should be mentioned.
✓ What can be improved in the future also should be mentioned.
Before applying this principle, I used to see bad pull request descriptions. Ex:
✕ “Fix build”
✕ “Fix bugs”
✕ “Add patch”
✕ “Moving code from A to B”
✕ “Kill weird URLs.”
Those descriptions aren’t clear. What bug? What Patch? What did you do to fix it? It’s a lot harder for the reviewer to understand the context.
For a small PR, mostly it’s not a big problem, but Imagine you have to review a large PR like the screenshot below:
3. Make It Visual
Add some screenshots for your changes! Screenshots simply make the job for the reviewer much easier. It’s easier to reason about front-end changes when you can visually see what’s been changed in the PR. If you’re feeling extra generous, add a GIF or video of you using the feature to add some extra validation for your UI changes.
—
Pull Request Template
In order to keep every engineer in Mamikos aligned with the principle.
We set up the PR template on Gitlab, so Engineers just need to answer a few questions when they’re going to create a Pull Request:
- “What issue/feature/bugfix/improvement did you solve?”
- “How do you solve the issue?”
- “Approach’s Shortcomings”
- “Things need to be improved in the future”
- “Unit Test Result”
- “User Interface Screenshots”
Discipline is the key
We always encourage all of our developers and new joiners to follow the principles above because it’s really important to our development process and able to improve to quality of our codebase.
This is also the reason why Open Source projects out there have a good quality. Most of them are because they always have great discussions on every PR, and when a contributor doesn’t explain anything in his PR, it’s usually impossible for the PR to be approved.
I hope this article can inspire you or/and your company to adopt the same principles.
