On Code Reviews in 2021
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.
The example code review is finally available here: https://medium.com/@ExplosionPills/an-example-code-review-490b2418475c
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. For example, there may not need to be any discussion after pointing out a misspelling in the codebase. 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 author’s changes.
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 the 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 of all but the simplest of changes. I prefer this grateful and inquiring 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 also to learn and teach potential improvements that can be applied to a broader scope later on and even in future projects. My boss reminds me of one of the core tenets of Mobiquity code review:
Be Kind.
How To Code Review
This section won’t go into the technical aspects of code review. Instead, it will discuss a general approach to performing code reviews, 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 and other disciplines, 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 a 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 accept 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 and not one that is defensive. As a reviewer, you should have the grateful and inquiring mindset and not one that is antagonistic. Reviewers may make mistakes in their review as well, and these can also be important learning experiences.
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 one 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. It may even be appropriate to tell the author that they should push code for review more often and in smaller, logical chunks.
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.
Leaving Feedback
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 future development, reviews, and ultimately, standards.
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 depth of the solution, and they will inevitably see it differently than you do. In the end, they may understand it more deeply. You also need to remember that your proposed solution may not be optimal or even better than what was already written. 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 and need to make corrections.
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 details 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.
Implementing Feedback
Code reviews are often looked at as pass-fail gates for introducing code changes into a project. Existing version control and merge request tools, and the software development process itself can make it difficult to accept some code changes in a merge request while rejecting others. Thus, recommended changes are often split into two categories:
- Changes required to accept the merge
- Technical debt items that will be refactored in the future
Cynics can point out that technical debt items will never be done, but I still think it’s possible to learn lessons from feedback even the changes themselves aren’t implemented. As a reviewer, part of your responsibility will be to move development forward with an appropriate level of quality. It’s a judgment call that takes practice and is impossible to do perfectly.
In the past, I have accepted code changes to move the project forward in contemplation of refactoring at an appropriate time. Then, I’ve provided my own merge request with refactoring suggestions to be reviewed. I think this is a good approach, although it’s difficult to do very often since it requires code stability, among other things. Not all reviewers may have time to refactor in this way as well, but it is an approach to code review to keep in mind, and you can always do it on a small scale and as an example.
Reviewers and authors must be willing to compromise and decide on the optimal outcome of the code changes based on their constraints. Achieving this takes time and practice, but it’s a necessary part of the software development exercise. If you are a reviewer who is not having your code changes successfully implemented, you may want to revisit the feedback that you are leaving. Your goals may be overreaching or even incorrect. In this case, it may be better to adopt the standards of the team.
On the other hand, authors must pay attention to feedback that they get consistently. Don’t resist refactoring or implementing suggestions only because of a time crunch or visceral disagreement. Make sure that you understand the technical debt that will be created if you don’t address it now. If your code is correct, you may be able to improve its clarity or documentation as well to reduce the red herrings of reviewers in the future.
The ongoing communication between reviewers and authors that is engendered at least in part by code review will help them improve in quality over time. Authors may improve their code writing in general, and reviewers will learn better ways to give the team feedback. You and your team must work together to find the best way to learn from and implement feedback at the appropriate time.
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 enterprise. 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. A meta analysis of code reviews can help provide feedback for individual improvements or a chance to improve the standards. Reviewers won’t have much opportunity to improve if there is no appraisal of their work. 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. Stabilization and refactoring efforts should be applied not just to the code itself but to the general process of software development including code review.
Common Pitfalls
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 the coding. As time goes on, especially if work becomes difficult and timelines draw near, the quality of any of these ostensibly 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 than 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 a team’s slip in their code review quality. We’re only human, after all, but the slip 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 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 without addressing the feedback.
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 go poorly. 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.
Summary
I will reiterate my definition of code review: it is an open and ongoing communication between authors and reviewers about the authors’ code. I will also restate the goal:
Code review maximizes the number of people who fully understand the code and alters the code to achieve its purpose optimally.
The code review itself can be considered a report that reviewers create in response to authors’ code. More broadly, code review is a subprocess inside of the larger software development process that is a collaboration and communication between reviewers and authors which often includes discussions, changes, documentation, technical debt items or other issues to be tracked, and much more.
Reviewers and authors should approach code review with a grateful and inquiring mindset. This will allow both to learn and grow, and the code will be better off. After a review, the reviewers should be able to fully understand the authors’ intent, and all should concur that it aligns with whatever the stated goals of the code may be. This process is an integral part of software development, and this can’t be understated. You and your team will greatly benefit from a strong understanding of how to code review properly and a hardening of your code review process.
