How To Complete Great Code Reviews
My introduction to code reviews
I remember my introduction to code reviews. I had just been moved to a new team and my task was to go through a project’s entire code base and fix the spacing. In every file. It was tedious and it took forever. There was only one very busy senior developer doing code reviews so it took days for reviews to happen. Comments included derisive remarks. My code changes would be rejected simply because the reviewer noticed something unrelated in the file he wanted changed. This was a great demonstration of how a code review should not be done.
Since then, I’ve been amazed to learn that code reviews aren’t actually intended to be torture. They’re a great tool for developing the skills of individual developers and giving team members overall visibility into a project, as well as the obvious benefit of maintaining and improving the codebase. Code reviews foster communication and help ensure that a team doesn’t get into a situation where only one person is familiar with a piece of code. As noted above, it’s definitely possible to do them wrong, so here are some best practices we use at Bounteous to make code reviews a beneficial experience for everyone.
Keep it positive
In many ways this can be the biggest challenge of a code review. Peer review can be stressful for team relationships if the benefits aren’t obvious. It’s helpful to see feedback as a learning opportunity and a way to improve code quality across the team. For reviewers, this means being mindful of tone when pointing out faults. Code review isn’t just about finding faults – it’s important to ask questions and give positive comments, too.
For developers it means not taking comments personally, which can be hard. Developers should feel free to question feedback, ask for explanations and avoid falling into the trap of making changes just because they were told to do so.
Lastly, code reviews shouldn’t be seen as performance reviews. Let me repeat that – code reviews and feedback are opportunities for learning and growth. It’s in the best interest of each reviewer and reviewee to get the most out of it.
Standard for approval
There should be established standards for what does and doesn’t pass a code review. It’s a good idea to have a checklist (https://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/) that everyone can refer to and to keep up to date. Establishing standards keeps approval from being blocked by things like formatting issues or differences of opinion over equally acceptable solutions. The general guidelines will be the same for every project (the changes should compile and work correctly, for example) but different projects may have special needs that should be noted.
Some people are better at code reviews than others, and that’s okay. The only way to get better at doing code reviews is to do them. Code reviews should be a regular part of every developer’s workflow. This has a few benefits – the work is spread out so everyone can do short sessions frequently, and reviews can happen in a timely manner. Additionally, having everyone do code reviews prevents an “Us vs Them” mentality between submitters and reviewers.
Review all code
Even if the change is very simple or you’re a senior developer there should always be another set of eyes on changes before they enter the code base.
Automate common issues
Review sessions should be short and frequent
A study done by Smart Bear Software at Cisco (http://support.smartbear.com/support/media/resources/cc/book/code-review-cisco-case-study.pdf) showed that for optimal effectiveness developers should review fewer than 200-400 lines of code at a time and fewer than 300-500 lines of code an hour. More than that and the ability to find errors goes down. Take enough time for a proper review but never review code for more than 60 minutes at a stretch.
Know what’s being reviewed
The reviewer should know the purpose of the code they’re reviewing, which means the developer needs to provide that information. It may be appropriate to link to Jira tickets or other related info. The reviewer isn’t going to complain about having too much info. Developers should also also be in the habit of annotating their code to clarify their intentions in case they’re not obvious. The review should be limited to the the scope of the actual task. The code review isn’t a time to add new requirements. If the reviewer notices other issues outside the scope of the current review those should be addressed separately rather than delaying the original changes.
Now you know how to avoid some pitfalls and establish a positive, effective code review practice. By establishing standards for acceptance, ensuring that everyone reviews code (but not too much!) and that all code is reviewed, automating the common issues. Following these simple guidelines has helped Bounteous provide a framework for code reviews that improves code quality, fosters communication within teams, and helps all our developers improve their skills and make code reviews a beneficial experience for everyone.