Once upon a time, a developer created a PR and requested for review.
Little did he know:
- He misspelt a variable name.
- He forgot to remove a
console.log("testing").
- He overlooked a
// TODO: refactor this code.
As such, it took several rounds of review to get it across the line.
Had he reviewed his PR first, he could've avoided this disaster.
What is a PR?
A PR is basically a request of "May I put some code changes into your branch?".
In a collaborated environment, the main branch is usually protected. It means that you need to create a PR and obtain an approval to contribute your code changes.
Why should I review my own PR?
One may say, "I know my code and it works. Why should I review my own PR?"
To answer that, let me introduce a new made-up word - Approvability. :rocket:
Achieving "It works." is the first step of creating a PR. Getting someone to approve it is the goal.
From a reviewer's point of view, it's much easier to approve an easy-to-follow PR than one that exhausts their brain power with ambiguous function names and obscure logic.
Apart from approvability, self-review can:
1. Eliminate stupid mistakes
Trivial mistakes like typos and code misalignment can make yourself look unprofessional.
2. Enhance readability
If you have a hard time reading your code, the reviewer will have an even harder time reviewing your code.
3. Improve code quality
When you review your own code changes from a bird's eye view, you may come up with refactoring ideas. :bulb:
How should I review my own PR?
Review it like a PR from someone else.
Ask yourself:
1. Does it satisfy the requirements?
When we're so focused to make the code work, we could miss some of the requirements.
We don't want the tester to come to us and say "It doesn't work!", right?
2. Does it exceed the requirements?
When we code, we may see things that we want to refactor (or fix). Very often, those refactoring are unrelated to the requirements.
It's called scope creep. The result is a PR with two sets of changes entangled together, making it harder to review.
When it happens, consider splitting the additional code changes to another PR. It will make your reviewer's life easier. :relaxed:
3. Do I need to think twice to understand the code?
If yes, it means that the reviewer will need to think 3+ times.
When it happens, we have 3 options:
- Refactor the code to make it more readable
- Use code comment to explain the "why" (but not the "what")
- Use PR comment to invite discussions
4. Does the commit history make sense?
Reviewers may rely on commit history to follow your train of thoughts.
Before requesting for review, you have the opportunity to tidy up the commit history.
After requesting for review, do not force push.
Thank you for reading.
Hope you find this post interesting!
How do you review your own PR?
Share your experience in the comments!