This article is an introduction to code reviews and covers the main principles and some common pitfalls. It is intended for developers (regardless of level) who’ve just started participating in an existing code review process but don’t quite get it. Though it still may be useful for a seasoned reviewer as some of the pitfalls are tricky to overcome.
The idea of code review is pretty simple: have other people review the code after someone wrote it. I don’t want to dive into specifics of the organizing code review process here, as most teams already have an established flow (and this is a topic for a different article anyway).
At a bare minimum, an author provides a code change to a reviewer, and they exchange comments. (You can do this in person, too.)
If you’re a student, try this on your next group project.
To be good at code reviews, you must understand what their purpose is.
Code review has numerous benefits. The first benefit—and the most obvious one—is catching bugs early. The math is simple: the earlier you catch a bug—the cheaper it is to fix.
You might have experienced this effect in your practice. If a bug manifests itself after changing just a few lines of code, it is usually straightforward to debug and fix it. (It must be somehow related to the two lines you’ve changed.) On the other hand, if you find a bug weeks after it was introduced and tons of new features were added, the process is usually much harder.
Let’s lift this example to organization level: fixing a bug that has been deployed to production and detected by users. The user reports an issue to the support. Support (or several levels of support) handles it, verifies the issue exists and opens a ticket. The ticket is planned, which involves a manager and might involve the whole team. A programmer reproduces, debugs, and fixes the bug. QA engineer verifies the patch fixes the issue. This whole fixing process now involves at least 5 people and could easily cost a company thousands of dollars.
Detecting and fixing bugs earlier is orders of magnitude cheaper. That’s why techniques as TDD (Test-Driven Development), code reviews, CI (Continuous Integration), static typing, and linters are so good.
Detecting and fixing bugs earlier is orders of magnitude cheaper.
You should also apply the team thinking here—think of the benefits for the whole team or the company, not your own. One of the common complaints is “What’s my benefit from spending an hour reviewing someone else’s code?” The benefit is that spending an hour reviewing the code could help another developer avoid five hours debugging later on, so the project might get released sooner. Code reviews are an investment—you might not see an immediate profit.
There are also other benefits to code reviews.
- Exposing yourself to lots of new code is a great way to discover new ideas and learn new techniques.
- Open discussions—if done right—improve team communication and build company culture.
- Having at least two programmers to understand every piece of code builds shared understanding and reduces code ownership.
Many teams report that learning, teaching, and socializing far outweigh finding bugs. I see this shift as a natural part of the team’s growth as the team gets more experienced with peer reviews.
Now that we know why code review exists and have a full buy-in for it let’s see how to do it. Again, I don’t want to dive into specifics as they may vary significantly from team to team, but I do want to give some tips that are always applicable and will help you to avoid common pitfalls.
Receiving Code Review
Self-review and self-test the code before submitting it for review.
That’s a quick step, but it will help you avoid very obvious mistakes such as committing in wrong files, commented-out or debug code, unfinished TODO items—you’ll be surprised how often this happens. Make sure the code passes all tests locally and on CI.
Doing this will save your and reviewers’ time and reduce the number of required follow-ups.
You can also take it one step further and think of how you could improve the code, factor out common pieces, come up with better names, or add helpful comments.
It is your responsibility to get the code merged.
That means it’s you who must find and follow-up reviewers. Pick people who are familiar with the changed code. Check if they can do a review soon enough; if they are not available—pick another reviewer. If you don’t get a review on agreed time, ping the reviewer.
Code reviews are not personal.
That is one of the most important ideas in the article. Getting it wrong is the surest way to destroy your review experience.
One of the pervasive things even experienced developers do is that we identify ourselves with the code we write. We treat every bug as a personal shortcoming. We believe we write perfect code and don’t want to know how many bugs there are. Finding a bug in our code is like finding a bug in us as humans. It is difficult to accept criticism of the code calmly if we view the world through this prism.
You need to disidentify from your code—your code is not you. You should also accept that making mistakes is a part of our job—it is normal, and even the most senior developers make them. Don’t be critical of yourself if someone finds a bug in your code—consider it a “good catch,” not a personal failing.
The idea is easy to understand, but it might be hard to accept and live by.
To make matters worse, most peer reviews are conducted in electronic form, and it is incredibly prone to misinterpretation. We as human beings also tend to misinterpret messages in a negative way: if a message is questionable, we assume the worst. The good news, it is easy to fix: talk to the sender in-person (or over the phone) and clarify what they mean (protip: they don’t want to degrade you).
If your feelings get hurt, back off and cool down before replying—you don’t want to rush into conflict. It is useful to remember that it is code who gets reviewed—not you.
Social and psychological issues of code reviews is a big topic. Read an excellent Humanizing Peer Reviews by Karl E. Wiegers for more information (also linked in the “[Next steps]” section).
Giving Code Review
If you commit to reviewing code, review it thoroughly.
Teams doing code review usually set a rule that no code can be merged without being reviewed first. When people do not understand the importance of code review, and they want to get the code merged faster, they might want to cut the process and ask for a quick “+1” from you.
Don’t give in! You know code reviews are important and super-cool—now educate others!
When doing a review, stay focused. Don’t review too big chunks at a time, and don’t review for more than an hour straight—your focus will diminish, and you’ll miss a lot.
You may review the code of more qualified team members.
If you are a junior developer may feel like you are not qualified enough to review the code of the more senior developers. You are! We all make mistakes, and most of them are very stupid ones.
Also, remember that one of the reasons we do reviews is to learn; to learn better, read the best code out there.
This is a mirror to Code reviews are not personal. Remember that you are reviewing the code, not the author. Avoid any comments that describe reviewer (or may be interpreted as such).
Be constructive in your comments, rather than critical. Ask questions, rather than make requests. That opens discussion instead of commanding.
Focus on how code can be improved instead of how it is broken.
Avoid sarcasm (it is extremely easy to misinterpret).
Remember that bugs are the bad guys in a review, not the author or the reviewers.
Philipp Hauer gives more examples for providing feedback in his Code Review Guidelines.
To get the most of this reading, don’t stop here. Completing this section requires about an hour of your time.
Read Code Review Guidelines by Philipp Hauer (~5-10 minutes).
In his guidelines, Philipp brings attention to human aspects of code reviews and proposes a “humble” mindset for author and number of phrasing techniques for the reviewer.
Watch Implementing a Strong Code-Review Culture, RailsConf 2015 (31-minutes talk).
In this talk, Derek Prior stresses the importance of code reviews for socializing, learning, and teaching. He also gives excellent tips on how to do good code reviews.
Read Humanizing Peer Reviews by Karl E. Wiegers (~20 minutes).
It’s an excellent article on the social and psychological aspects of code reviews and associated barriers. Well worth reading.
Examine a couple of the last reviews you’ve made (if any); consider how would you do them differently now.
Do not skip this!—I hope you’ll experience a difference in your mindset (and your behavior).
I did this after writing the article, too. Well… the good news is that there is a room for improvement.
Reviewing code is more of an art than a science—the only way to learn it is to do it.
Protip: You may review any code.
Reviewing code has too many benefits to do this upon request only. You may review any changes without an invitation—you’ll get a better understanding of the project, learn new things, and may catch a bug or two.
I have deliberately limited the article to only include bulletproof techniques—they can be applied by anyone and are guaranteed to work. There are many more profound tips I can share, but I want to keep this article beginner-friendly and of reasonable length.