Code Review and your team

Let’s say you’re on a small software development team — either within a department or in a smaller organization. You may or may not be doing code reviews, and you may or may not think they’re important (hint: they are).

As a software engineer and software engineering manager, I think that code reviews are one of the most important and valuable things a development team does.

Types of Code Reviews

Code reviews can range from no reviews to formalized code review meetings with review documentation.

• At the least strenuous, is No Code Reviews. This is exactly what it sounds. Developers commit their code and move on to the next task (Note that this is not to say they do or do not test their code, this is focused entirely on reviewing the content of the code changes).

• In a Self Review culture, developers review their own code when finishing up a feature. This helps catch silly mistakes such as committing files that shouldn’t be versioned and acts as a basic sanity check.

• A step up from this is calling over others to review a developer’s code at their desk before committing it. This is also known as a Desk Check and gives an external perspective to code, but the reviewer is constrained to a short period of time and doesn’t have full freedom to look over the code.

• Increasing in formality, a developer can send code off to others for Peer Review at their leisure. This can be done by E-Mail, a source control system such as GitHub or GitLab, or a formal code review tool such as Upsource.

• Drastically escalating the formality, teams can convene a Code Review Meeting between two or more individuals in which the changes are discussed as a team.

• Finally, at the more extreme levels of formality, a formal Code Walkthrough is conducted where the changes are reviewed line by line and notes are recorded on defects and other issues found.

This is a scale of formality in code reviews with more formality not necessarily being better. The appropriate level of rigor depends on the system and team in question. Some important factors include:

• The magnitude of the change
• The sensitivity of the System / Severity of a Failure
• Skill level of the team
• Knowledge level of the team
• Code Quality in relevant areas
• The maturity level of the product offering
• Size of the user base

Teams don’t necessarily need to stick with the same process for each change. A massive refactoring of business-critical systems likely warrants a heavier degree of scrutiny than fixing a Null Reference Exception.

The Value of Code Reviews

Certainly, the value of code reviews comes from finding bugs, enforcing a common code style, and the very act of knowing a code review will occur increases the quality of code committed, but there are other benefits as well.

Code Reviews improve the quality of your code, but they also improve the quality of your team. By involving others in code review, you spread knowledge of the application to others. If a developer goes out on vacation or is tied up on another project, it’s more likely that reviewers will be able to get involved in that project as well. Additionally, if a bug occurs in production after a release, the developer and also the reviewers have a chance to remember the changes made, so involving others in the process increases the team’s capability to respond to emergent work.

Developers will also pick up on tricks and practices from others by reviewing their code and can incorporate these practices into their own code. More experienced developers will teach younger developers new language features and libraries to make their lives easier and help them continually learn. Additionally, different developers tend to have different focuses. A performance-oriented reviewer is going to give entirely different feedback on code than an architecturally-minded developer or a security or testability-minded developer.

Getting the most out of Code Review

I’ve found a few key takeaways for getting the most out of code review. Most of this is aimed at small to medium-sized software as a service (SaaS) organizations since that is where the bulk of my experience is, but these practices should be beneficial to many other types of organizations.

In no particular order:

For the vast majority of reviews, sending a peer review request on to one or two reviewers is sufficient. For larger and riskier reviews, either the submitter or reviewer could request a more formal review process. Alternatively, a team could have a few hard and fast rules such as:

• If there are over 250 lines modified, it needs an escalated review process
• If certain files or methods are touched, it needs an escalated review process
• If the change is only to application data, it requires a lesser degree of formality

When assigning reviewers, I like to assign a subject matter expert to the review as well as someone either on the more junior side or new to that application. This helps spread the knowledge around and gives a safety net.

When actually reviewing code, I like to look through code in a few passes. First, I’ll look over the code as a whole to look for the files changed, types of changes made, and the overall shape of the file. Next I’ll look at the code again reading for what specific changes are being introduced from a functional standpoint. After that I’ll look at architecture, maintainability, and code styles. I’ll often finish up with testability, performance, and security passes. By forcing myself to look at the code through a series of lenses, I can trick my brain into focusing on things in new lights and finding things I wouldn’t find otherwise.

I like to manage code reviews via merge requests and include details in the description as to:

• The business logic around the change
• Any technical debt, architectural, or testability changes
• Any alternatives that were considered / not gone with
• The developer’s plan for testing the change

That last one is particularly important as a code review should validate or find fault with the developer’s plan for ensuring the code change does not introduce defects. This could be as simple as unit tests included with the change or something like “I tested this screen in IE and Chrome and as guest and authenticated user by working with feature X, Y, and Z”

Overall, code review is an extremely useful way of improving both your code and your team and a vital way of spreading knowledge, experience, and continuously improving the quality of your code and skills of your team.