Code review is not a new topic, but is for sure one of the most overlooked activities in software development.
As software developers we are a team of people that try to get stuff done, and the best teams that get stuff done have open, collaborative cultures based on feedback. For a well functioning team it’s vital that the feedback comes fast, because the cost of fixing a problem in code rises exponentially the further that problem goes undiscovered in the lifecycle of the software.
It’s important that teams put in mechanisms to find the problems as early as possible when it’s most cost effective to fix them.
The fastest way to get feedback regarding the quality of the source code is automated tests. The problem is that it’s time consuming and expensive to increase test coverage of the existing code.
Another way to improve code quality as early as possible is to use static code analysis tools like SonarQube or CodeClimate.
But there’s the human factor of code review that cannot be replaced. It’s the activity where you ask your peers for code review because, unlike any code analysis tool, they understand not only programming but also the business of your application.
Motivation
Code review is maybe the easiest way to improve the source code quality, early in the development process.
Besides that, it spreads code ownership and makes team members aware of the way the source code changes.
Nonetheless, it’s a good way to mentor developers by collaborating on the source code.
Code review process
There’s no silver bullet when it comes to code review. It depends on the size of the team, on the level of the peers, on the complexity and nature of the source code. But there are a few guidelines that work on any team.
The first rule is to consider code review a mandatory step in your Definition Of Ready.
The second rule is to review everything that cannot be automated.
Auto-formatters, style-checkers and static code analysers should be in place so that developers do not waste time with trivial issues.
The third rule: Do it!
Everybody should be involved in the code review process. Of course, it is impossible that everybody gives feedback on every review, and this is not desired. But it is also wrong to have only one developer in the team – usually the most senior one – do all the code review.
Next, let’s see how an effective code review process looks like.
The branch
Once a developer starts working on a story, she created a new branch for that story. So it’s crucial that the story is well written so that the code changes to implement are minimal.
Also, it’s important that the branches are short lived and every feature is merged into the codebase as soon as possible.
The merge request
When the code is ready, the developer should ask for a merge request / pull request. There are many reasons why code review should start online, but let’s list just the two of the most important ones:
- It allows peers to decide when to review, without interrupting their ongoing work.
- It enables collaboration for both co-located and remote teams.
The good part is that any modern version control system like Github, Gitlab or Bitbucket facilitates code review through merge requests. There are also dedicated collaborative code review tools you could use, like Gerrit, Crucible, FishEye or Phabricator, but for most teams the VCS is enough.
When the developer asks for code review, she sets herself as owner of the review, and her peers as reviewers. If the code changes affect a lot of files, it’s sometimes better to merge request as soon as there is something to review, mentioning in the merge request description the TODOs left. This way, the reviewers have more time to give feedback, coming back to this merge requests once files are changed. Anyway, the user story should be small enough so that the number of changed lines is minimal.
The review
Once the merge request is created – or the review is created when using dedicated review tools – peers can begin to give feedback. It’s important for reviewers to focus on finding problems in reviewed code, issues that static code analysis tools can’t address.
All the questions below should be answered by code review:
- Does the code meet the requirements? Did the author implement what the business needed?
- Is the architecture correct?
- Is it future-proof?
- Are the coding idioms correct?
- Is the code reusable?
- Does it respect domain knowledge?
- Were automated tests implemented for the code under review?
Sometimes, review has to be taken offline and discussed live or in screen-share session. This is normal and it usually happens when critical code blocks are under review.
The merge
Working on feature branches allows development teams to test features even before merging them into the main development branch (usually master). Reviewers can checkout the code under review and run it locally. Moreover, if your continuous integration pipeline is set up correctly, you should be able to deploy the reviewed code on one of your testing environments.
Once the code has been reviewed by the minimum number of reviewers, the author can merge the feature branch. Usually, for a scrum team, a number of two reviewers is enough.
Tips for better code review
- Automate what can be automated. Use style-checkers and static code analysis tools.
- Don’t take code review personally. The author should embrace feedback. Reviewers should formulate decent comments. Good reviews often use words like “please”, “thank you”, “maybe it would be better”…
- Ask for a review for small changes made in a hurry, as sometimes that’s where the ugly bugs live. Developers often omit to ask for review on small changes, because they wrongly consider the changes are too small to be reviewed.
- Treat configuration as code and include configuration changes into the review process.
- Focus on finding problems, not on finding solutions. Sometimes reviewers tend to offer solutions instead of finding problems. Code review is not about rewriting the reviewed code, it’s about finding problems in the code as soon as possible. Pair programming is the right activity for finding solutions collaboratively, not code review.
Final thoughts
Following good development practices is important even if it slows down the production. It’s important to improve the development process to ensure quality, team cohesion and collective knowledge. Code review is the activity that – if done right – requires minimum effort for maximum effect. Oh, and keep in mind LeBlanc’s law: Later equals never.
– Robert Sicoie
Software Architect,
Java Department
