What is code review?
Code review is a process where an author submits their work to be reviewed by other developers and vice versa.
This process is used to ensure that the code is of high quality and it follows the standards set by the team.
Besides helping to keep a high-quality codebase and having clean code organization,
it also helps with something more important, which is learning and knowledge sharing.
I have worked on different teams and I have seen places where there are no Code Review processes, at other times developers use Code Review as a way to prove that they are right and the other person is wrong. Later in the article, we will see which approach is the best that big companies developers do it.
If there is no harmony in the team, no good result will come from it.
Real-world scenario
There are 8 steps in a code review process that developers go through in companies before it gets to production, these steps differ from company to company but most of them are as follows.
- The author submits the code for review
- The reviewer reviews the code
- The reviewer gives feedback
- The author makes the changes
- The reviewer approves the changes
- The code is merged into the main branch
- The changes are deployed into staging or pre-production for testing.
- The changes are deployed to production
Some companies do it that at least two people review the new Pull Request before accepting it. Others do it by randomly selecting team members. Bigger companies did put more effort into this and they build AI tools that decides who can review the code for your Pull Request.
How to approach a code review
The majority of the developers are used to reading PRs from top to bottom because that's how we read everything, some try to do it from bottom to top, but neither worked for me.
After reading about how other companies do it, I found out that identifying the most important part of the PR first is the best way to go.
The best approach is to look with a Broad overview, then find out the Important parts, then focus on details.
1. Broad overview
Although to identify the important part most of the time you need to know the project, but for small projects should be more straightforward.
While scanning for the changes and identifying the more important parts, there are many tools that help you mark them or collapse them so you can focus on the important parts.
2. The important part
Once you have a good idea of what is being changed you should focus on the important parts of the code. Here you take your time to check if everything is correct by focusing on the business logic and requirements and not the details. This is the difference from the details part, here you should not focus on the code quality, but on the business logic and requirements.
3. Details
While you made sure that the important parts are correct, you can now focus on the details. This is the part where you should check if the code is well organized if the code is easy to read and understand, if the code is following the standards of the team, and if the code is well documented. Make sure the tests are covering the new code, and that the tests are easy to understand and maintain. Make sure that the code is improving the codebase quality and not making it worse.
The checklist
I will write a checklist that you can use to go through a PR and make sure that you are not missing anything.
# Broad overview
[] Identify the important parts
# The important part
[] Are the requirements implemented correctly?
[] Is the business logic correct?
[] Is there any edge case that is not covered?
[] Are the tests covering the new code?
# Details
[] Is the code well organized?
[] Is the code easy to read and understand?
[] Is the code following the standards of the team?
[] Is the documentation updated?
[] Are the tests easy to understand and maintain?
[] Is the code improving the codebase quality and not making it worse?
[] Do the names make sense?
Automating the process
Automating the Code Review process is also something that helps you with the Checklist above, for example if you look for tools that do static analysis of your code, bug finders, etc. Then you can configure and set up in a way which your team has agreed upon.
Checkout this Github Repot to see a thorough list of tools you can use for PHP projects. After implementing tools like above you will end up having a very simple checklist where it lets you focus only on the important parts.
# Broad overview
[] Identify the important parts
# The important part
[] Are the requirements implemented correctly?
[] Is the business logic correct?
[] Is there any edge case that is not covered?
# Details
[] Is the documentation explained well?
[] Are the tests easy to understand and maintain?
[] Do the names make sense?
Commenting on a PR
Before giving feedback you should make sure that you understand the code, and that you have a good idea of what is being changed, but most importantly be curious about why the author made the changes. When giving feedback you need to be polite and respectful, keep in mind that the author of the code is a human and your teammate. Also, be clear and specific, and try to explain why you are suggesting the changes, be open to feedback as well, and you should be willing to discuss the changes.
Changing the team's perspective
This is important that if you trying to do a change in the team, you need to be on the same level as the team, you need to understand how they feel about the topic, and what they know about it.
By making sure everyone is on the same page and is on the same level, then you try to build a plan together.
Checklist
[] Gather feedback from the team.
[] Make a detailed plan.
[] Present the plan to the team and ask for feedback.
[] Keep track of the team following the plan.
Below you will find a list of resources where you can also read more in detail about code review processes on how they affect the team speed and team morale.
Consider this as an orientation and not a goal, because there is always room for improvement.
TIP: Google has found that it's very important for the individual responses to come quickly than it is for the whole process to happen rapidly, so instead of keeping the author waiting to write a comment when you will review it.
TIP: When creating a Pull-request, that is fixing a bug or adding a feature, don't make a lot of refactoring on top of that PR.
- It delays the changes that might be urgent to deploy.
- It adds more pressure on the reviewer and harder to review.
- This should not stop you from doing refactoring changes, just add them on a different Pull-request.
When adding a feature just add the feature and nothing out of that context. -
The smaller the better.