Code Review: The Most Important Conversation For Engineers

The “quality” of a codebase is difficult to define. Unlike precious metals or jewels, there is no standard rubric. Understanding quality requires experience and intuition, and even experienced software developers can have wildly differing opinions on the quality of the same code. Nonetheless, every good engineering organization strives for a high-quality codebase.

“Why yes, this 2.5-carat Django app is a beauty!”

Most engineers can agree on some basic principles for quality of a codebase. It is often best understood from the perspective of a new team member. When first diving into a codebase, a new engineer might ask themself:

How easy is it to navigate?
How consistent is the structure?
Are the functions well-named and easy to read?
Is it idiomatic?
Is it extensible?
When reading it, do I need to take a break every 10 minutes and get a snack?

It is easy for a team to ignore code quality. Hastily written code often works, even if it is poorly structured. The costs of ignoring code quality are often only felt in the future, which can make it hard to prioritize. A system can also be well-architected but badly coded. Thoughtful design and planning are important for large decisions, but are far too blunt an instrument to address code quality.

Bad code is also viral, since most additions will be copied rather than written completely from scratch. When a single bad pattern multiplies, a frequently-developed project can go downhill quickly. Once this happens, the team must allocate time to fix many problems instead of dealing with the root cause in the first place. Stronger developers will often fix code that they happen to be working on, but this is a slow and inconsistent process. And even if every member of your team is an expert, inconsistent standards can be just as difficult to navigate as junior-dev spaghetti code.

Improving Quality Through Communication

Here at Knewton, we emphasize communication and collaboration: “Learn and Teach” is one of our core values, and it extends to our team as well as our product. The only way to enforce good coding standards is to talk about the code your team actually writes, and code review is the best way to make sure that conversation happens. A high-quality codebase is the result of high-quality commits, and code review ensures that the quality of every commit is high.

Code review is the only chance that engineers get to communicate about the code that they actually write

It can be difficult to introduce code review to a team, and even harder to introduce effective code review. “Mandatory code review” is not enough; superficial or stubborn code reviews are actively harmful, as they waste time and breed resentment. A team needs a good culture around how code reviews take place.

Here are a few guidelines we use at Knewton for good code reviews:

• Code review is not just for style, it is also for approach. The reviewer should think about the requirements and evaluate the design decisions made by the author.
• Code review is part of the work in a ticket, not an afterthought. A ticket is not complete until code review comments have been addressed (and the code is merged and deployed).
• Code review is not personal: authors should expect reviewers to nitpick and ask questions, and reviewers should be encouraged to give detailed feedback while still treating the author respectfully.
• Code reviews should iterate minimally. Code reviews become a time-sink if you need to chase down reviewers multiple times for the same commit. Reviewers should be thorough in their review, responsive to questions or concerns, and then move on.
• Code review should block merges, not releases: poor quality commits should not be allowed in the codebase.
• As a rule of thumb, on any medium-sized commit (100+ lines or so), it is unlikely that there are no comments to be made.

For even more reading on good practices, Scott Nonnenberg has a great guide to common code review antipatterns.

Communication Makes Everything Better

In addition to improving quality, code review is a fantastic way for engineers to learn and grow. Short of pair programming, code review is the only chance that two engineers have to talk directly about how the code is written, and is the best way to propagate good instincts across the team. The best education comes from failure, but making mistakes is a costly way to learn, and hiring experienced engineers is difficult. In lieu of this, the next best way to learn is to work with other engineers on a well designed system.

Code review also encourages personal interactions among team members. Showing your code to a colleague requires vulnerability. Regular code review builds trust and empathy, and handling constructive criticism makes team relationships stronger.

Additionally, code review exerts backpressure on the software development process: knowing that your design will be scrutinized discourages taking sloppy shortcuts, and makes you more likely to collaborate on your approach ahead of time to avoid having to redo it.

Finally, code review speeds up development time in the long run by reducing technical debt. Having a readable and well-factored codebase makes it easier to add new features and onboard new engineers. Most teams operating without reviews fret that a review process would slow development, but this concern is usually overblown. Even if an early code review requires substantial discussion and iteration, engineers will quickly coalesce on best practices, and early mistakes will not be repeated. As an added benefit, by reviewing code, engineers become more familiar with a codebase, allowing them to respond to production issues faster and make major changes more confidently.

Get The Conversation Started

The vast majority of software startups have too little review, rather than too much. Starting a new process is hard, especially when it is inserted into your daily process of writing new code. Make sure the whole team is on board before taking drastic measures like blocking all merges on code review.

As with any process, teams must remain vigilant to make sure they are not wasting time over-polishing their code. A good rule of thumb is that each commit should get a single, thorough review from one developer familiar with the project. The marginal value of a second or third review is small, and a reviewer without context is going to waste their time.

It is important to recognize that code review is more than simply a tool to prevent mistakes. When engineers are forced to agree upon the best way to write a solution, they quickly become a stronger and more productive team. By spending time on the nitty-gritty details, your team can quickly resolve their differences and move on to the larger and more interesting problems.