By Maja Matic
Introduction
Code Review is the process of examining written code with the purpose of highlighting mistakes and possible improvements. Usually, when we start working on a ticket we create a new branch. After the ticket was implemented and verified, we create pull requests to let our team members know that we completed our ticket and give them the possibility to review the code and give us valuable feedback.
What is the purpose of code reviews?
In the survey, 682 developers have been asked on what they spend their productive time. From the graph, it is visible that 45% of the productive developers time goes on Technical Debt and fixing Bugs. This is a huge number and it can be drastically decreased by adopting a regular review process.
So, back to the question what is the main purpose of reviewing code?
The most common advantages of adopting a code review process include:
- Finding bugs and security vulnerabilities on time
- Creating performant software
- Strengthen the teamwork
- Creating high-quality software
- Creating well-documented software and readable code
- Adherence to coding standards
- Creating maintainable and scalable software
- Simplifying solutions and reusing already existing code
- Teaching and sharing knowledge between developers
- Creating software that is well tested (unit tests, integration tests)
When done correctly, code reviews can reduce the amount of time and effort involved in testing, as well as in investigating and fixing bugs found after release. Reviews can also save money, by finding bugs that could harm or lead to an unsatisfied end user. Code reviews are also very important for team communication. They encourage developers to talk to each other, distribute ownership and share their knowledge and experience. Through code reviews, developers evolve habits of writing cleaner, scalable, secure, well-documented code that in the end leads to a higher quality of software. Code reviews are a very useful process in software development and every development team should consider including them into their daily processes.
How do code reviews work?
The code review process involves developers examining source code that they personally were not involved in writing with the aim of giving valuable feedback to the author. The feedback given is constructively critical and should be used to improve the knowledge sharing inside the team and strengthen the communication skills in the team. Ideally, the reviewers should not have been directly involved in the author’s specific implementation. This leads to objective feedback that should focus on code readability, sustainability, and documentation. All the feedback received from the reviewer should be considered and discussed.
As developers, we should keep in mind never merge code prior to the review and especially never push code to stable branches (for example master). In our team we follow a simple rule: when something needs to be changed in a stable branch and it is more than one line of code, there should be a branch with a pull request created. In case the change is only one line of code, we are allowed to push to a stable branch but need to inform the team about our commit, so they can recheck (review) it.
In code reviews, developers can also consider including non-coders, so that for example, the Graphic Artist or/and Game Designer get the chance to see the feature before it gets merged to a stable branch.
Best Practises
Code reviews can be very time consuming but also very frustrating for the author and the reviewers. Sometimes it is very hard to get people to review your code since they are busy with their own tickets. A very common frustration is also to get code reviewed by team members working in different time zones. Especially in cases where the author didn’t prepare the code review well, there can be a lot of comments caused by misunderstandings which can lead to long and frustrating discussions.
To avoid looking at pull requests as a burden and rather see them as improvements of yourself and the code quality, there are some best practices every developer should be aware of.
Preparing code for review
It is very important that the author of the code prepares a good code review to not waste the time and motivation of the reviewers. So, what does it mean to prepare your code well for a code review? Preparing code for a code review can have different meanings in different teams. Let’s mention some of the best practises and explain them in detail.
Pull Request Templates and Repository Configuration
When working in a repository we should create pull requests based on templates. These templates represent points which should be done before giving your code to other reviewers for review. It is basically something similar to a checklist, where the code author can check if his code is well prepared for a code review. This would include points like the scope of the code review, test coverage, documentation updates and many others which will be mentioned further below. There are possibilities to create these templates in the most popular version control systems. For example, when using GitLab, we just need to create a .md file inside .gitlab/merge_request_template/.
It is also very useful to have repository configurations like status checks and merge rules. Status checks would, for example, be when we have a continuous integration running, which checks if tests are passing. This way we make sure that nothing gets merged which is breaking the tests. It is an important part of the code preparation, because in case that the authors forgot to check if the tests are still running, they will see it through the status check and can update the changes before giving the code for review. Merge rules can also be very useful, especially when working in teams where the team structure is split into backend and frontend developers. The code can be reviewed by any developer inside a team, but with merge rules, we can enforce that a specific developer or group of developers need to review the code before it can get merged. This can be useful in situations like when for example, a frontend developer creates a pull request which can be reviewed by any developer in a team, but they expect at least another frontend developer to review the code. The merge rules give also the possibility to define the minimum number of developers that need to review the code before it gets merged into a stable branch.
Code review scopes and sizes
Changes should have a self-contained and well-defined scope that they cover exhaustively. Well-defined tickets tend to generate small code reviews. Having code reviews that include a lot of files and take a long time to review have a bad impact on the review quality and the time dedicated per section will decrease. So, it is important to create appropriately sized pull requests. In situations where the author of the code realises that the code changes that need to get reviewed are too large for one code review and take a long time to be reviewed, they should consider splitting them into multiple self-contained code reviews.
Submit complete, self-reviewed and self-tested Code reviews
It is important that the author of the code rechecks his code before giving it to other reviewers to not waste their time. This means that we should only submit complete, self-reviewed (for example, by diff) and self-tested code reviews. We should test the submitted changes to make sure that the code is working fine (running test suits) and make sure that our code changes are fitting all code quality checks. For the most code quality checks, we can use automated tooling, which can take care of unit tests, code style checks, and others.
Refactoring should be separated from behavior changes
It should be avoided to do refactoring and code formatting changes in the same code reviews as behavior changes. Doing both in the same code reviews leads to touching many lines and files which will lower the reviewer’s attention. In the end, a lowered attention can lead to creating leaks for unintended behavior changes into the code base. So the best practice would be to separate the behavior changes and refactoring into different code reviews. For style checks, syntax and formatting debates we can use automated tools, which will reduce the expensive human review time spend on issues like that.
Clear commit messages
It is very important to have clear commit messages so the reviewers actually know what this code changes are about. We should be more explicit on what and how something was done. It will help the whole team in the future to understand why something was changed, but also help the reviewer to easier understand our code changes. Below is a common example of a bad commit message and a commit message that is considered to be a good one, since it is not only describing what was done but also how it was done.
A bad commit message would be:
Review fixes
A good commit message would be:
[TicketNumber] – To fix the frozen resource display we register the tooltip component on a child element since we should never register two components on the same element.
Enhanced code reviews
Code reviews can be enhanced by adding images and/or videos to them. This can be especially useful when working on UI/UX changes. Sometimes the code can be reviewed by developers who can’t visually imagine how a code change would affect the UI/UX, so it is very useful to include a screenshot as an example. This can help the reviewer to visually see the change and maybe provide some better and simplified solutions. There is this saying that “A picture is worth a thousand words”, which also applies when it comes to code reviews since some changes might not be obvious.
Sometimes it can also be very useful to enhanced code reviews with a video. This is especially very useful when working in teams that include team members from different time zones. With videos, the author has the ability to walk reviewers through the code and explain why something was done the way it was done. It can also be very helpful for reviewers to explain their comments because written communication can easily be misunderstood.
Responding to reviews
Good code reviews are not only based on good code review preparations, but also on the review process itself. Following points, explained in detail below, are important to keep and develop a healthy code review culture.
Separate time to do reviews
As a reviewer, we should start as soon as possible with reviewing code changes so we can make sure to give valuable feedback and give enough time for discussions and review fixes. It is very important to make a habit of regularly checking if any of your coworkers need a code review. It can be very frustrating for the code author when their code gets reviewed in the last second and they need to do all the review fixes in a short amount of time. We should keep in mind to be fair to our team members and make a habit to spare a few minutes a day to review the code from our coworkers. In my experience, it is usually very useful to do code reviews before starting to work on something new and probably every morning before starting to work. Usually, it helps to check the open pull requests during stand-ups and encourage people on reviewing the open pull requests.
Give code examples and respect the scope of the review
When suggesting a different approach it is always helpful to include code examples. As mentioned before written communication can be easily misunderstood, so code examples are perfect to explain what exactly was meant by a written comment. These code examples can be pseudo-code based. They will give a higher understanding and avoid unnecessary discussions.
As reviewers, we should always respect the scope of the review. There is always the possibility to improve the code base, but we need to consider if those improvements would still be in the scope of the review. Improvements can and should be pointed out and in situations where they don’t fit the scope of the review, there is a possibility to create follow up tickets.
Give valuable feedback and be nice
We should always keep in mind that there is a human on the other side of the code review, so please be nice. It is always good to give positive feedback, to point out that something was done well, to increase the motivation of our coworkers, but also to point out the good parts of the code.
There will be situations in which reviewers will need to criticise the code. It is important to keep in mind when criticising, it should be done the right way. A good approach is to form comments as questions and not demands. So instead of writing a comment like “Extract the service to reduce code duplication”, which sounds like a command, it would be much nicer to write something like “What do you think about extracting a service to reduce code duplication?”. This way the reviewer gives the authors the possibility to explain why they did something a certain way and opens a possibility for a conversation. When suggesting approaches it is important to describe what, how and why we would like something to be changed. The suggestions should be based on software principles and not on personal opinions.
Both positive and negative feedback should be very valuable. The code author should not get offended by the reviewer’s suggestions and the suggestions should be taken seriously even if the code authors don’t agree with them. As code authors, we should respond to every comment in our pull requests so the code reviewers don’t feel like being ignored. In some pull requests, we have the possibility to create tasks (which can be checked after changing/implementation) or even just a simple reply as “done” can already be enough.
As mentioned earlier there will be situations where the code author and reviewer won’t agree on the same solution, which can easily lead to conflict. To avoid huge written discussions in pull requests, the developers involved in the code review, should recognise these situations early and instead of continuing the written discussion, concentrate on finding a solution through verbal communication. When they finally agree on a solution, the solution and the explanation why they agreed on that solution, should be put into written form to the pull request, so the other code reviewers will get value out of it too.
Conclusion
Always keep in mind to be nice during code reviews. Code reviews are very important for knowledge sharing but also for strengthening the team, so try to make the best out of them to motivate your coworkers, to share knowledge, but also to keep the code nice and clean.
In 2018 I attended a great talk “Pull Requests for everyone” by Catherine Meade at JSConf Iceland. Catherine closed her talk with some amazing and motivating words about code reviews which motivated me to do the same:
“Giving positive feedback is an easy habit to adopt. You’re already reading through the lines of code, looking for issues. It doesn’t take much time to throw in a ‘Nice!’ or ‘Looks great!’ If you see something awesome, say it!” – Bryan Braun
More Information about this topic
Talks
- Pull Requests – Merging good practices into your project by Luca Bezerra at PyBay2018:
https://youtu.be/35nf46MJRTY - Implementing a Strong Code-Review Culture by Derek Prior at RailsConf 2015:
https://youtu.be/PJjmw9TRB7s - Pull Requests for everyone by Catherine Meade at JSConf Iceland 2018:
https://youtu.be/LqOcx-CtN0w
Internet sources and books
- Humanizing Peer reviews by Karl E. Wiegers:
https://www.processimpact.com/articles/humanizing_reviews.pdf - What is Code Review? by SmartBear:
https://smartbear.com/learn/code-review/what-is-code-review/ - Code Review Best Practices by Palantir:
https://medium.com/palantir/code-review-best-practices-19e02780015f - What to Look for in a Code Review by Trisha Gee:
https://leanpub.com/whattolookforinacodereview - Best Kept Secrets of Peer Code Review by SmartBear:
https://smartbear.com/resources/ebooks/best-kept-secrets-of-code-review/ - Illustrations by Maxim Priakhin
Anna Sunderbrink 
Andreas Hackel