Modern software development is complex. New technologies emerge at a breakneck pace. Best practices, patterns, recommendations, and samples are a dime a dozen. Any team larger than one is faced with how to address differences in style, knowledge, and discipline. Perhaps one of the most important tools in your arsenal here is an an effective code review. Let’s look today at some ideas on how to code review effectively.
What is a code review?
In order to understand how to code review effectively it is important to also know what a code review is. For brevity, a code review is the act of having peers review one another’s code consciously and systematically (see also Smartbear’s article). In essence, when you as a software developer write code you seek peer review of said code.
Code reviews on the surface may seem like just dotting your I’s and crossing your T’s. It is so much more than that, however. They are also an opportunity to provide coaching, mentoring, and guidance. Frequently they allow junior engineers to show something new to senior engineers. If done correctly, they are not about seniority at all.
Some source control managers (SCM) have built-in support for code reviews. For example, in Git there is the concept of a Pull Request (PR). Open source projects thrive on community and this is heavily facilitated through individuals who make changes and submit PRs.
Other SCM tools, however, may or may not support this. TFVC, for example, allows you to require and mark a “reviewer” on a check-in before it commits the changes. SVN, on the other hand, doesn’t have any built-in mechanism to enforce code reviews.
Why should I perform code reviews?
There are a myriad of reasons why you should be interested in code reviews. I doubt I could possibly enumerate all of them. That said, I will try to touch on what I feel are some of the most important reasons for having code reviews.
Having effective code reviews a part of your workflow can be imperative to keeping a clean repository if done correctly. This is an opportunity for developers to keep stewardship over the code to prevent new technical debt, discuss alternative and/or better ways of implementing features, and more.
The ultimate goal in a code review is to ensure good, clean, and well thought-out code is entering your repository. Another extremely applicable point from the agile manifesto is that you should favor interactions and individuals over processes and tools. In this regard the code review is only a tool to help achieve this interaction and communication about your code.
It’s *not* to prove how much smarter you are
One challenge with code reviews is that some people use it as a gate. What I mean by this is that inevitably there will be “that one” engineer who thinks they are smarter than everyone else. Whether they are or not really has nothing to do with it. For them a code review is an opportunity to show how much smarter they are and put down or demean others. This is absolutely not the right way to perform a code review.
It is not just to check a box
Another common problem is that it becomes routine. When individuals on either side of the code review are not committed to improvement, the code review process can become rote. If you’re having a code review simply to check a box then you’re doing it wrong.
How do I initiate a code review?
Given the importance we place on a code review it is equally important to understand how to initiate them. Depending on the source control manager (SCM) you use (you do use one, right?), this may actually not be trivial.
I previously mentioned that Git supports something called a PR. This is the most effective way to require and track a code review.
If your SCM of choice does not have a code review mechanism built-in, you may have to look into 3rd party options. If a 3rd party tool also does not exist, worst case you may have to institute a policy.
What makes a good code review?
In order to have an effective code review, here are some points to focus on:
- Small iterations. SmartBear performed a study that revealed you should review no more than 200-400 lines of code at a time.
- If you, as the author, feel anything requires explanation you should annotate it ahead of time.
- Provide specific and actionable coaching about the code changes. This can and should be bi-directional. It is just as important for a senior engineer to have junior engineers review their code as the inverse.
- Refer to agreed upon styling, naming, structure of code, and conventions. Learn the difference between required and preference. Just because you like it one way doesn’t mean the other person should do it that way.
- Watch for potential introduction of technical debt and ways to reduce and/or avoid it. You may not be able to avoid technical debt in all circumstances but you should always be identify and discuss as it arises.
- Discuss code coverage. Are there new unit tests with the code? Do the tests pass? Do they cover all the bases?
- Ask questions. A PR should be collaborative. This means it is ok for the reviewer to ask questions and to seek clarification.
- Don’t make it personal. Don’t take it personal.
Code review scenario; coaching on how to coach
I’m writing this blog post today in part due to a real-life scenario I recently witnessed. The context of the situation is important to frame the story. We’re undergoing a transformation within the company. We have years of history and development in several TFVC collections. The current and previous development teams have more or less been reviewing the check-ins (by requiring a reviewer, etc).
My team is working on migrating code to Git and setting up a CI/CD pipeline as part of the transformation efforts. Given that, we block writes to master and require PRs for merges. I’ve been monitoring that repository all week. While perusing the pull requests I came across one that hadn’t been approved yet. Clicking into the PR pleases me to find some conversation. However, I’m sad to see the exchange going on.
There are a couple magic numbers in the changes. The reviewer accurately calls it out and requests the submitter to remove them. His comment simply states: “remove magic numbers”. The engineer responds with an explanation that seems justifiable at face value. To me it appears a stalemate. Why? Because reviewer did not provide specific and actionable feedback.
I decided to take the opportunity to do two things: 1) coach the engineer on how he can improve his code and remove the magic numbers 2) coach the code reviewer on how to better communicate to the engineer his thoughts.
It was that particular pull request that helped me understand some of the transformation efforts we still have ahead of us. This, to me, is all what a code review is about. It is not to point out what is wrong but rather to suggest ways to strengthen and improve the code. Also, it is about teaching each other how to improve communication.
Conclusion
Learning how to effectively code review is important for your success as an individual as well as a team. Do not use this time to show how smart you are but use it as a collaborative session. Code reviews are a great opportunity to provide bi-directional coaching and mentoring.
Additional Reading / credits
It is impossible to fit everything into a post. I invite you to peruse some of the resources I reviewed before constructing this post.
- Google: Engineering Practices Documentation – How to do a code review
- SmartBear: Best Practices for Peer Code Review
- Digital Ocean: How to Conduct Effective Code Reviews
- Perforce: 9 Best Practices for Code Review
- Palantir: Code Review Best Practices
- Gergely Orosz (Stackoverflow): How to Make Good Code Reviews Better
