A code review seems to be a fairly simple concept. You read the code of your colleagues, comment on it (and vice versa) and thus improve it. Thanks to code reviews, you don’t just create software that works – you create something of real quality. A good code review takes time, but it’s an investment that easily pays off for the product and the people working on it.
There are many ways you can approach a code review. You can evaluate the code in pairs, during pair programming, or in a more formal way – most often by creating merge requests to the develop branch, which may be merged only after the acceptance of the code for the rest of the team (or at least part of it).
So what’s the payoff?
First of all, it makes programmers learn something new. The flow of programming knowledge helps to produce better code, and teaches new skills. It’s profitable for both seniors and juniors.
It improves code quality and reduces the number of errors in the production environment, thus saving the work of testers and developers that would later repair your bugs.
CR requires code to be easily readable for other developers, so the introduction of new people to the project is much easier.
Thanks to suggestions of good practices and design patterns from senior developers, the code is well-prepared for the addition of new features and modifications in the future.
Some general rules
- Everyone should do code reviews, regardless of level of seniority. It is beneficial for everyone in the team. You learn how to write a good code and implement it, so you can develop new features more easily.
- No change that hasn’t been evaluated by – preferably – two people should come into the production code. We thus reduce the risk of major errors in the production code.
- It’s best if code is evaluated by members of the same team. It’s much easier to review the code of a project you work on, as you’re familiar with the product’s functionality and specific technologies.
- Don’t think of the CR as a competition for the best code or time for kick-in-the-butt, harsh assessment. It’s only a tool that helps to create good software, so don’t be afraid of it.
- In addition, your CI should check if the code builds with the change, the tests pass, it follows the styling guide etc., because CR shouldn’t focus on those issues. These tasks are repeatable. People shouldn’t bother themselves with those. The focus should be on stuff that only a human can understand.
What NOT to do
These can make a CR a waste of time, or even harmful:
- Don’t underestimate the CR: don’t do it without focus. It’s as important as creating code by yourself.
- Don’t mock or humiliate your peers. It’s just rude and spoils the atmosphere in the team.
- Don’t write sloppy code just because you know that all mistakes will be picked up by the CR.
Checklist time!
Reviewee’s responsibilities (yeah, they’ve got them too!)
Let’s also consider what our responsibilities are as the person who prepares the changes to be reviewed.
- Changes should be small – a big lump of code takes a long time to read and is much more difficult to understand, so the smaller the change, the better
- If there are additional changes (improvements to the code), this should be mentioned in the description of Merge Request
- Name branches properly – make it consistent with the ticket you’re working on and give it a clear name
- Name your commits in simple yet meaningful way (a commit named „Fix” is maybe simple, but definitely NOT understandable)
- Read the code before CR – this simple habit can save you from putting something really stupid on your Merge Request.
As reviewee, you also need to remember a few important things:
- Don’t get defensive, accept valid remarks. It’s nothing personal – better code makes your future work easier, so think about it – again – as an investment.
- Discuss invalid remarks. You don’t have to agree with all comments – constructive discussions can create better solutions.
- Don’t be afraid to ask for help or clarification. On the other hand, also try to write understandable comments when you review someone else’s code.
- Read upon unknown concepts. If you find phrases and concepts that you’re not familiar with, look them up, do some research. It’s your time to learn.
Code reviews are a broad topic and this article provides only basic information and advice. If you want to learn more, I encourage you to read:
https://blog.jetbrains.com/upsource/2015/07/23/what-to-look-for-in-a-code-review/
Code Reviews – Just Do It
Best Practices for Code Review
