This article is about the code review best practices. It explains code review from the common-sense perspective. If you do not want to read through the whole article take a look at the table of contents and the summary for a quick overview.
A code review is a process in which a team of developers reviews and critiques source code changes before they are committed to a project. The goal of a code review is to identify any mistakes or issues with the code, and to ensure that the code adheres to the project’s coding standards and best practices.
Plainly put, code review exists to prevent bugs from ending up in your production. But, it is much more than that. It has technical aspects, technologies, expertise, knowledge change, and feelings involved.
If you are the person being reviewed you want to know what can you do to make your code more understandable. On the other side, if you are a reviewer, you want to make a positive impact on the reviewed code.
It is a process, which has a goal to assure the quality of delivery. In this specific case, the team is reviewing the quality of the code. This is accomplished by one or more developers examining the code which their colleagues wrote.
The quality of code review is hard to measure. However, a good code review should be able to accomplish several items.
There are several steps involved in a typical code review process:
Overall, the code review process helps to ensure the quality and reliability of the code in a project, and can also be a valuable learning opportunity for developers.
First of all, a good code review is friendly, polite and it aims to help improve code and product or feature this code implements. Secondly, it is a process where knowledge and experiences are shared. It is where developers get dedicated time to communicate, raise concerns, consult, and try to discover problems before they happen.
A good code review is event where developers:
More importantly, a good code review is not, or at least it should not be, a place where anyone tries to demonstrate any kind of domination or does any ill-intended actions. It is a consultancy, an unwritten agreement between developers to help each other by double-checking what was done.
Therefore, a good code review is thorough, constructive, and focused on improving the code and the development process. Here are a few characteristics of a good code review:
Overall, a good code review is an objective, constructive, and thorough evaluation of the code that is focused on improving the code and the development process.
Before starting with a code review, we should consider several important ugly truths.
Ticket planning should include the time needed for a proper review. The team should be aware of something called Definition of Done. This is a list of things that should be completed before the ticket is considered as finished.
Review tasks should be part of DOD and review time should add to that ticket. A team should calculate the cumulative expected time for this ticket. If 3 developers spend 2 hours that is actually 6 hours for code review, which is 1 man/day of the sprint.
Original developers should test reviewed code before inviting others to review it. This is important because it will save everyone’s time. Developers will always make bugs. The only question is when will the team discover these bugs.
Managers will not always have an understanding of the time needed to do a proper review. It is up to developers and reviewers to communicate this properly. Other team members will not always have an understanding of long or complicated reviews.
People make mistakes and that is completely all right. Bugs might appear in code, even after a good review.
Furthermore, we are aware that different reviews can have different weights. For example, some reviews will be easy, while some of them will be hard.
Let’s try to group these code reviews in categories, so we can establish code review best practices.
Code reviews take time, sometimes lots of time. Multiple developers are reading code, spending time and focus on it. Management sometimes does not understand why this investment is important.
Sometimes, code reviews can block development. A common example is when one feature is not completed because something complex is discovered during a code review. This is blocking other features etc. However, this might indicate that planning was not done in a good way.
Different people do code reviews with different motivation, spirits and quality. We are not all equal and we should not be. Capable manager or team lead should know how to leverage their team and build code review process to focus on strong sides of team, instead on weak sides.
Code reviews are improving quality of code, reducing future technical debt and improving overall quality of software. Furthermore, they are in a way building a team, people communicate, share opinions, concerns, fears, advises and faith.
Without code reviews there would be less quality, structure and teamwork. Even time savings would not be there, because that time saved on code review is already lost and reserved for fixing issues and sorting our overlooked problems.
Reviews won’t happen in a minute, it takes time to do it properly. The team should plan proper time blocks, depending on the complexity of the review.
Determining a type of review early is extremely important because it provides us with information on how to handle it. Furthermore, we should be able to compare our capabilities to properly examine the provided code.
Finally, if the review is too challenging we might decide to invite another reviewer or even ask to be additionally educated on technical topics from that review.
Code review is also called the pull request, I am using both terminologies in this article, depending on the sources that I am referring to.
Ben Balter, a Senior Manager of Product Management at GitHub, wrote a fantastic post about The six types of pull requests you see on GitHub. This post classifies pull requests as:
Furthermore, Ben described how each of these types works and when to use it. For example “just a heads up” would be a simple (one-liner) pull request where the author doesn’t need review.
On the other hand, line-by-line is pull request which is used when product is ready to be shipped.
There is no denying that pairing is a highly visible cost to management: “Two developers at one workstation? No way! We already pay them a fortune!” – Paul Blair,
The Unnoticed Costs of Code Review by Pull Request
Firstly, when we examine new pull request, we need check number of changed lines.
If a pull request has more than 500 lines author of that request should consider splitting it into multiple parts or find a way to get involved in the code review process.
Furthermore, reviewers should check if tests are executed for big pull requests.
Otherwise, reviewer might find critical issue in the middle of review, and then everything would go back to start.
There are two ways to consider impact of a pull request:
The second one is the important one.
When measuring impact of pull request, team should think about several things:
But wait, should the developer or reviewer really be concerned about these items during the code review?
No.
Because these things should have been defined when a ticket is created and planned. For example, during the sprint planning phase, or even earlier during sessions between product owner and management.
However, developers should not close their eyes if they notice any of the mentioned issues.
Code Review Best Practices Checklist:
I can not stress this enough.
It is important to be friendly, professional, and polite during the code review process. The bigger the company, professional communication is more important.
It does not matter if you are the person who is reviewing or the one being reviewed. Keep an open mind, accept any and all comments with dignity and grace. Always think of what you can improve and what others might benefit off.
Don’t try to be ironic or angry. If some comment is important clearly state it. On the other hand, if something is just your opinion and it does not hold objective improvement state that as well.
Execute any available tests for feature which you are reviewing.
Think of manual (quick tests) which can be done. For example, if the feature changes the database connection function, simply try to connect to the database. If your feature is changing the BasicController component for API, just try to execute a couple of different API calls
I was already writing about clean code rules and how to improve the quality of your code. By improving the quality of your code in general your skills will get better both as a code reviewer and as the one being reviewed.
Different developers use different coding editors. Some companies have policies about this and some don’t.
Therefore, it is expected that some syntax errors might crawl up to a pull request.
What does this means?
Don’t examine pull requests only trough pull request interface (i.e. bitbucket or gitlabs). You can also check out these branches and examine them in your trusted editor.
Most of the companies have some document with coding standards or best practices.
If such document exist, pull request should include comparing the latest code with these best practices.
If there is no such document, here are some quick things to check:
Depending on pull request and type of the change some software design patterns can be applied.
Also, if some design patterns are already inside code architecture they should be applied.
For example, adding a new function in all inherited classes of an abstract class would not make sense. Furthermore, having a strategy pattern in place and not creating a new strategy for some configuration would also be a wrong usage of that design pattern.
Your software will always have legacy software.
Meaning, your pull requests will affect this software somehow.
Therefore, you need to handle this. 🙂
I was already writing about clean code rules and how to improve the quality of your code. By improving the quality of your code in general your skills will get better both as a code reviewer and as the one being reviewed.
The code review is an extremely important part of the whole software development process. Therefore, the whole team must make enough effort to properly plan the time needed for this.
Pull requests differ from case to case, but teams should always be aware of impacts and potential issues.
Mistakes will happen to those who work. Code review is essentially risk management, therefore it should be treated as such.
A good code review will improve the overall quality of the code, but it will also reduce the risk related to a new feature.
API design is an important aspect of modern software development. It allows different systems to… Read More
This article sheds some light related to the question will ChatGPT or AIs in general… Read More
This article provides an overview of new features and deprecations in PHP 8.2. PHP 8.0… Read More
This article is about Automation and Artificial Intelligence in Software Engineering: Experiences, Challenges, and Opportunities.… Read More
PHP is getting more and more features. Enumerations in PHP are one of the latest… Read More
This article writes about what is new in PHP 8.0. These are personal notes related… Read More