Code reviews still rule

Searching-512Recently at InfluxDB we discussed how code reviews fit in during the various stages of development. It’s great to see the team reach consensus about how we should develop software. It made me think more deeply about why I remain a big believer in the code review process.

After the compiler, and unit-testing, code reviews are the next best thing for finding errors in your code.  That it improves quality of the code being reviewed is obvious, but the benefits of code review go beyond that.

Spreading design knowledge

Reviews are a powerful way to spread design knowledge within a development team. Learning a new code base can take time and discipline, and reviews are an effective way to make it happen. While at TiVo, the team and I practised a lightweight code-review process we called an “OTS” — or an “over-the-shoulder” review. It involved a quick walk-through of the change by the author with another engineer. Even these lightweight reviews helped the team deepen their understanding of the systems we built, and this increased understanding helps later debugging sessions so much.

It also helps spread expertise within your team. I’ve spent the last few years at very small software companies, and senior engineers don’t have much — if any — time for mentoring their junior colleagues. But code reviews are an implicit way for senior people to make sure the more junior members of the team improve their skills.

Builds consensus

Coding style and idioms are among the most contentious issues a development team can face, and yet can waste significant time. Code reviews can help a team reach consensus more quickly about how to structure the code, as various changes are discussed.

Helps hiring

Developers want to work with other developers they consider talented and professional. Code reviews are the professional thing to do, and create an excellent impression, particularly of an open-source project. We on the InfluxDB team review almost all changes we make, and other developers notice:

What not to review

Developer time is a finite resource, so it’s important to know what to focus on. While it’s nice to review unit tests, I don’t insist on it.

Hopefully unit tests accompany all changes to the production code, so changes to test code are naturally reviewed with the production changes. Changes to Makefiles and builds script are not something that need constant review either.

Think it’s expensive?

Try not doing it, and see how expensive that gets. Watch team design sessions take longer as design context has to be constantly provided. Debugging starts to takes longer and longer as a smaller and smaller fraction of development team understand the code.

If you and your team are not reviewing your code, start doing it. It may feel slow in the beginning, and it may feel your understanding of the code base means the reviews are shallow, but that changes quickly. The impact on a team of shared commitment to the review process has enormous benefits, well beyond simple code quality.

4 thoughts on “Code reviews still rule”

  1. I couldn’t agree more. Deep code reviews are the primary tool to maintain code consistency, quality, and maintainability. Plus all the benefits that you get from mentoring and knowledge sharing. Because you are basically re-solving a problem and thinking about alternatives and if that solution is optimal, it’s not easy, and code reviews do take a significant amount of time and mental energy. What I found for myself, it not only makes me a better code reader but also keeps me up to date on the code changes.

  2. Great article, Code Review is indeed a great tool for improve software quality. It’s also a great to tool to embrace application security.
    Being able to identify application security issues (those that are not caught by unit test although some could) and let developers know about why they are a security risk, will definitely add a LOT of value for the development team and so continue working on having software resilient to attacks.

Leave a Reply

Your email address will not be published. Required fields are marked *