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.

Everyone participates

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

You know who’s great at reading code? Computers. There are automated tools that can be added to the development workflow that will save time and effort during code reviews. For example, JavaScript Linting allows identification of common code issues during development so those get fixed even before the code review happens. This frees the developer up to focus on actual development instead of typing, and prevents reviewers from having to point out common issues. Automation has the added benefit of preventing developers from feeling nitpicked and prevents the situation where functional but poorly formatted code is blocked from approval.

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.

Conclusion

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.