How to review a Pull Request
The nuances of checking other people's codeDecember 09, 2019
Pull Requests are the heart of any team working on a shared project using git. It lives in that particular time when the code has been finished but not shipped yet. In this void, QA and pull request reviews happen.
Understanding other people’s code is part of every developer’s job. Reviews are the perfect moment to train this skill.
Before a code review
A review is not a request for a complete rewrite, nor an exercise in pedantry or a meaningless thumb up.
There are a few things that have to be assumed when giving a review to avoid miscommunications.
What follows are general assumptions that work in most cases. Every team should have its own discussion on what is assumed and what is not.
The code is working
One of the most important assumptions for a code review. A pull request review’s aim is to review the code, not to QA it. A frontend code review should not check styling or functionality. For backend, it should not query the service to see if the data is correct.
These steps should be their own process, they require more time and more people than a review should take.
The code is ready
Another vital aspect to take care of before a review happens is that everything is ready for it.
The code should be written in a way that the developer sees fit for deployment.
Code reviews aim to improve what is already in place. If the code is not fit for deployment, a review is just a waste of time for the whole team.
Taking more time to make sure the code reaches the standard of the codebase is not time wasted but gained. Your colleagues’ time is as valuable as yours.
The code is tested
Testing is another important aspect of every application. Untested code is fragile and unreliable. Yet it’s a practice I often see dismissed even in enterprise applications.
Making sure all new code is tested in a clean way is a huge first step in having a reliable and efficient codebase. So make sure every PR increases the total code coverage.
How to code review
A review is mostly a discussion.
It helps teams reach a shared expectation of what the code should look like.
- Could this be rewritten using another data structure to simplify the code?
- Could the code be split and abstracted?
- Is it better to adopt a more performant solution over a simpler one?
- Should the state of this component be global or local?
- Leading comma or trailing comma?
Don’t be afraid to start discussions around the code, that’s the purpose of a code review.
Experience does not matter
Everyone can give a good code review, regardless of experience.
Some of the best discussions in code reviews I’ve received came from juniors.
- The need for an explanation for an intricated part of the code can lead to a refactor to simplify it.
- A question about a silent agreement can lead to a push for better documentation.
At the same time, a senior developer can use a code review as a mentorship opportunity.
There are many ways to enhance a review as a senior.
- Pair programming code reviews can lead to teachable moments. You can go more in detail through the reasoning of the code. It’s even better than rubber duck debugging.
- Share resources! You might find that an idea you have is actually wrong or outdated. It can help teach a few things to others who did not come across the same resources.
- Document everything that needs answers to be understood. Build knowledge for your whole team.
Both more and less experienced developers can add value to reviews.
Never dismiss a review coming from a junior developer, and don’t be afraid to be the junior.
As a senior, use a review as a mentoring opportunity.
Reviews increase the overall quality of the code
If I had a cent for every bug caught in a code review, I would have already retired to a tropical beach some time ago.
Nobody can write perfect code. Try to review your own code a week after writing it, you will find plenty of things to improve.
Having a second (and third and fourth…) pair of eyes checking the same code is the best way to make sure the code is up to standard and bug-free. It also helps the long term maintainability of the codebase.
If you don’t have an extra pair of eyes, your own will suffice, as my teacher in middle school used to say, “Always read your test again before submitting”.
You don’t know how much your code can improve until you look back at it.
(Adopting Pull Requests) improved our stylistic consistency, gave us a forum to discuss code structure and architectural decisions, and increased the likelihood that typos and logical errors would be caught before they reached our users.
Code reviews can improve the code, documentation, and knowledge of the team.
Approach code reviews as both reviewer and code author with a willingness to improve. An ignored suggestion or unasked question is not going to make you a better developer.
Every voice in the team counts, every question deserves an answer in code, documentation or knowledge. Make sure to encourage and address them.