Back in 2017, I wrote an article called On Code Reviews that was both an open article on the topic of code reviews as well as some documentation for my then-and-still-current employer, Mobiquity, and its developers. I think that article is still relevant and has good information, but of course there have been tremendous changes in software development within the last four years which is an unavoidable aspect of a very fast-moving industry. I would not say that my opinions on code review have changed, although they have evolved. While that article is still a good read, it’s not necessary to read as a precursor to this article which should stand on its own.
After rereading the original, I also realized that I never filled in the section about web development specific code reviews. This article will only provide my general thoughts on code review, its purpose, and how to approach it, but this time around, I will write a separate article that includes specific examples based on a review that I did recently. My goal is that this article will change how developers see code review and how they approach it or at least reinforce its important aspects.
What Is Code Review?
The process of code review is nebulous and varies from organization to organization. In simplest terms, it is self-descriptive. However, this says nothing about how and when one should approach code review nor does it say anything about its goals.
Code review is an open and ongoing communication between authors and reviewers about the authors’ code. Usually there is only one author’s code being reviewed, and it is common for there to be one reviewer, at least at a time, although multiple reviewers are also very common. The mechanism for the communication can be anything, but frequently a code review is presented in the form of a report — inline and general comments about the author’s code. The report is an important and lasting aspect of the code review, but it’s not the only aspect. Reviewers and authors can have in-person, virtual, and text discussions about the code as well. These artifacts are more ephemeral, but they are not any less important.
The reason why I present code review as a communication will become clearer when I get to the purpose of code review. I think that communication describes it more accurately than discussion. While discussions do occur and are important to a code review, especially for large reviews where alternative solutions are presented, not all aspects of code review may be strictly discussed. I think that it would be a bit harsh to say that a discussion should be a required part of a code review, and especially in some small cases, the only communication that may happen is that the reviewer signs off on the code. How much are you going to discuss the correcting of a misspelling?
The Purpose of Code Review
The goals of code review can get lost in the shuffle as it is commonly baked into the process of software development for organizations or projects. In spite of this, we can’t lose sight of the goals or we risk the process becoming meaningless. Glancing at and signing off on code so that we can release a feature is not code review.
Code review maximizes the number of people who fully understand the code and alters the code to achieve its purpose optimally.
Code review is often thought of as an aspect of quality assurance, and it is, but discovering bugs, misses, and technical debt is a side effect of the core purpose of code review. If the code is understood better, issues in its quality become clearer. Reviewers typically perform the code review, but authors are an integral part of code review as well and should view themselves that way. After all, there wouldn’t be a reason for a code review if code to review hadn’t been written by the author.
Getting the most out of a code review requires getting into a particular mindset. As a reviewer, you should go into the code review…
- expecting to learn as much as possible from the author.
- expecting to teach the author as much as possible.
Thus, the best attitude to have when approaching a code review is one that is grateful and inquiring. No matter how senior the author or reviewer may be relative to one another, there will be something to learn or teach that can come out of the review. I prefer this attitude over one of a critical eye. While it can be very satisfying to catch logical issues in a code review, this may narrow your vision to the point where you miss the bigger picture. An open-minded approach allows you to not only catch possible issues, but to also learn and teach potential improvements that can be applied to a broader scope later on.
How To Code Review
This section won’t go into the technical aspects of code review. Instead, it will discuss a general approach on performing code review including how to get started and how to finish. It will offer suggestions for leaving feedback and documenting what was learned in an effort to help you and your team get the most out of your code reviews.
As with any other exercise in software development or otherwise, the only real way to improve is to practice. Learning new skills is always awkward, and that awkwardness can be a sign of growth. If you are new to code review, you shouldn’t let that fact discourage you from leaving feedback. I believe that developers should start code reviewing from the beginning. Of course you should support and supplement novice developer’s code review, but this is not to say that they can’t have great feedback. Even their questions and attempts to learn can help a very experienced developer see things a different way or reinforce good practices. If you’re a developer who hasn’t had a chance to review someone else’s code, you can and should practice by reviewing your own code. In fact, all developers should review their own code before they submit it to be reviewed by someone else.
You are bound to make mistakes no matter how experienced you are, and it’s important to be ready to make those mistakes. The collaborative nature of code review is what minimizes mistakes, not the talent of individual people. As an author, you should have the grateful and inquiring mindset, not one that is defensive. If a reviewer makes a mistake in their feedback, it can be a learning opportunity for both of you.
Starting the Review
Code review is not just a spot check, but a process. When getting ready to review code, you should start with the mindset described in the previous section: a grateful and inquiring open mind that is prepared to learn and understand as much as possible. You can approach it like you are sitting down to study or read to understand. Do the best you can to make sure you have the time and solitude necessary. Finish other outstanding work and try to eliminate other distractions.
If it’s feasible, work with another reviewer and especially the author. There is no substitute for being together in person, but this is often not very realistic. Virtual calls or availability on messaging apps are substitutes that will often have to do. The globalized nature of modern software development may also put heavy time constraints on team availability, so you may need to review a merge request when someone is inactive in order to meet a deadline. Keep in mind that code review does not require a merge request even though merge requests require code review. Your team should take time to review code when some or all of you can be together.
Depending upon the breadth of the code review, you may not be able to do the entire review in a single sitting. If you’re approaching the time you’ve allocated for doing the review or what you can mentally handle, get to a good stopping point and be prepared to pick back up later on or on another day. Keep the same startup approach when you’re ready to start again. Rushing a code review will work as well as rushing anything else including writing and testing code.
You can time box your code review based on what you are reviewing and what you think you can do. It may also be useful to select certain chunks or even files that you want to go through at a time and split those up to be reviewed at different intervals. Every developer is different, and you will find what works best for you through more practice and experience.
Your team should adopt a convention for leaving feedback on code. It’s very often that this is done through comments on merge requests which are a great tool. You may also want to document conversations about a particular merge request or code review meeting that you can refer back to later. Once you’ve completed this documentation, you can also synthesize it into lessons that can be applied to later development and reviews.
Feedback needs to be inquisitive. You should give the author the benefit of the doubt when you come across something you don’t understand. Your code review will likely consist of more questions than anything else. There may be satisfactory answers to those questions, but even asking the question might help the author see a better approach. When giving feedback in a code review or otherwise, I’m always reminded of Alexander Pope’s words in “An Essay On Criticism,”
Men must be taught as if you taught them not,
And things unknown propos’d as things forgot.
You are much more likely to get someone to adopt a solution to a problem if you get them to think that it is their idea. The point is not to be manipulative because that won’t work and isn’t a good long term strategy. Instead, you are trying to help the author see the full breadth of the solution. You also need to remember that your proposed solution may not be optimal or even better. As long as you have the grateful and inquiring mindset, you won’t be set back on your heels too much when you make a mistake like offering a suggestion that won’t work.
In simple terms, you can leave positive or negative feedback. The word negative is loaded, but in this context it would be any feedback that suggests a change to the original code. I have very rarely seen positive feedback on a code review, but the purpose it serves is the same. If you want to reinforce a practice that a developer has done well, positive feedback is a great way to do it. Even though people are prone to remember and act on negative feedback, they are also prone to pay attention to positive feedback. Positive feedback is often ignored by organizations in general. I could get into a deeper discussion about why this is, but instead, I’ll just end with this reminder: positive feedback on a code review is a necessary and useful tool.
A Good Conclusion
When you finish your first pass of a code review, your work is not done. In fact, you’ve introduced more work now because you not only need to go back and review the code again, but you also need to review your feedback. You should be prepared to read through the code multiple times in order to increase your understanding and offer more feedback. You may realize that you made a mistake in your own feedback as well.
I’ve seen a suggestion that code should take about three times as long to review as it took to write. What this really says to me is that there is no relative limitation on how long you can review code. As with books or any other media, you can consume them over and over and still find new things that you didn’t notice previously. For this reason, it’s important to define a stopping point for yourself as part of your preparations for the code review.
Being Realistic About Code Review
In an ideal world, everyone including myself would follow all of the advice that I’ve laid out in the previous sections, but if we lived in an ideal world, we wouldn’t have the problems to solve that make software development necessary. In fact, software development is frequently a very messy and disorganized exercise. While I present the advice above as an ideal to strive for and to look towards, in practice I realize that it is impossible for people and organizations to follow this consistently.
Working With Time Constraints
The first major blocker to optimal software development including code review is time constraints. Being able to spend three days reviewing code that it took one day to write sounds like a beautiful dream, but most reviewers have their own code that they need to write as well. A reviewer may also be focused on other tasks that they need to do while in the middle of an urgent code review which will inevitably lead to mistakes but is simply unavoidable.
Code review needs to be baked into any estimates of effort alongside other non-functional requirements like testing. It will be up to your team to determine how code review impacts your estimates. For example, in a story point model, a 3 point story might equate to a day’s worth of coding at your normal velocity and an additional day’s worth of code review. Perhaps you might even be willing to equate that to three days’ worth of code review too.
When there is breathing room, it’s important to take a step back and see how code reviews overall for your team or project are going. If the team is burned out, both the code writing and reviewing will suffer. Reviewers won’t have much opportunity to improve if there is no appraisal of their work overall. These breaks in the schedule can also be used for code review of existing code that is not part of a merge request. This can help you create technical debt items that you can incorporate alongside other fixes.
As projects go on, code reviews can become the victims of what is often thought of as entropy even if that may not be physically accurate. When a project is first started, there may be a lot of excitement about code review, attendance, testing, and other actions that are viewed as ancillary to actual coding. As time goes on, especially if work becomes difficult and timelines draw near, the quality of any of these so-called ancillary actions will be sacrificed to allow for more focus on the coding itself. Things just need to get done. The problem is that the quality of the code will suffer too which introduces more technical debt that can create a downward spiral in quality that can be more difficult to recover from if it were addressed in the first place.
Your team needs to have the mindset that all quality assurance exercises including code review are critical to software development. They can’t be sacrificed on the altar of coding because they are so integral. If you sacrifice them, you will have to pay for that cost elsewhere and probably further down the line. I won’t berate teams slips in their code review quality. We’re only human after all, but this is not an inevitability.
While in the weeds of a project, especially a difficult one or one approaching a deadline, it can be very difficult to see these sorts of quality slips. This is why rituals like the Scrum retrospective are so important. In addition to giving everyone a bit of a break and providing a forum for honesty, they give the team a chance to shine a light on these lurking quality problems. If code review has become perfunctory, it may be a good time to go through some training or introduce a third party to take look.
The usefulness of quality exercises diminishes when they are seen as more and more routine. A high quality code review requires a level of deep focus that is not always attainable, especially in high stress situations. There can be no doubt that developers get burned out reading code as much as they do writing it, and the quality of code review from both the author and the reviewer side can fall into the downward spiral alongside any other quality exercise. Eventually, it can get to the point where it appears useless, and developers merge code without offering any feedback, or authors that are tired of seeing seemingly the same issue over and over push for a merge.
Code review is only as useful as you make it. If you go in with the mentality that it’s a drudge that you have to complete because it’s part of the process rather than a quality exercise that is necessary for its own sake, it’s bound to not go well. If you can’t get into the grateful and inquiring mindset, free from as many distractions as possible, you may want to schedule your review for another time.
In that vein, I will get into the specifics of what you should look for and the kind of feedback you can provide in another article.