What is a code review? Well, it is exactly what you would think … a review of written code.
Does code need reviewing?
YES!!!
Think of it like asking a high school student if her English papers need editing. Most likely she will say no, but most of us know the answer is yes.
Code is like a rough draft of a paper. It needs to be edited and reworked. As hard as it is to admit, we can’t do it by ourselves. We as the code authors can’t review our code by ourselves. We need help … and that’s ok.
Why do we review code?
Why do we code review? Here are the four main reasons:
Better Quality
Like I mentioned above, code needs to be reviewed to find mistakes (logical, standards, design, etc.). One of the most beneficial reasons to do a code review is to improve quality, in all aspects of that word: quality of the code, the project, the user experience, etc.
Avoiding Code Heroes
Another reason to do code reviews is for knowledge sharing. When multiple people review the code, now there are multiple people who know what that code does. This makes it much easier to cut down silos and reduce the need for code heroes (a code hero is where only one person knows the code and thus has to be relied on to save everyone when there is an issue in that code).
Keeping Code Maintainable
The third most important reason for code reviews is to enforce code standards. It is always best to automate as much as you can to enforce code standards, but not every standard can be automated. By enforcing the standards in a code review, we help ensure the code is similar / readable / maintainable by anyone on the team.
Training the Next Generation
It also leads to the fourth main reason why we do code reviews: training apprentices. Because the apprenticeship program is so integral to who we are at RoboSource, we heavily rely on code reviews to train our apprentices – teach them how to (or how not to) write good code / solve problems / use good architecture. Some people wonder if code review is too late in the process to catch these issues. It’s not for us, because of how we break our work into small chunks (more on that in the next section).
How We Review Code
How do we do our code reviews? There are several key factors that influence the effectiveness of our code reviews.
Consistently
First, we perform them on a daily basis. Actually, it is not necessarily daily, but it is as needed (pro re nata for us Latin nerds). We make it a priority that every User Story we work on, every line of code that is written (by apprentice, senior devs, or even CEOs), is code reviewed. We try and break down our work into meaningful small chunks of work, and then code review that work. That means within a given week, most developers are submitting multiple code changesets for review.
With Tools
One of the reasons that allows us to do daily code reviews is that we do Tool-based reviews. We actually use the Atlassian suite of products (Jira and Crucible for this process). So, with the click of a button, a developer can create a code review from a User Story with all of her changes highlighted.
Crucible assigns the right people to the code review and allows us to create Change Items in Jira (our issue tracking system) whenever we see something that needs changed. One of the nicest features of Crucible is the ease in which a re-review can happen. After code review issues have been fixed, a reviewer will re-open the code review. Just the fixes are highlighted so that the reviewer can just review the new changes and sees them inline with the old code and code review comments. This tool makes it very easy for us to do code reviews from multiple locations and time zones.
By Keeping a List
Since we do a Group code review, we have found it very beneficial to also include a checklist of what to look for in a code review. Each person on the code review takes a specific lens (Standards, UX, Unit Testing, Completeness, Security, or Architectural). The lens focuses the reviewer’s attention on a checklist of things. This helps avoid Social Psychological issues like: Social Loafing (combatted because reviewers know they are the only one with that lens, so they need to do a good job); Inattentional Blindness (no longer do reviewers need to look for everything, but they can use Selective Attention and just pay attention to the important things on their checklist); etc.
The lens that is taken also helps set an expectation for how much time that person should spend on the code review. For RoboSource, our code reviews became much more efficient and effective when we started using these checklists.
Other Notes about Code Review
Important things to remember:
Don’t Let it be Personal
Code is not tied to a person’s identity. Tearing down someone’s code does not mean we are tearing down the person. This can be difficult at first, but in an open, candor environment, people do learn this lesson.
That being said, we try not to make code review comments personal (i.e.: “Why would anyone ever do this!”) but rather word most of our comments in question form, since we often don’t know everything that went into making a decision (i.e.: “Have you considered using this pattern here?”)
Review Everyone
Everyone gets code reviewed! Our first day apprentices get reviewed. We often tell our apprentices that they will get a lot of code review comments in their first few code reviews. But hopefully, as they learn our processes and standards, they will receive less general comments on their code reviews.
We also review our senior developers’ code. Often it can be intimidating for our apprentices to log comments or questions on a sr. dev code review, but we tell them it is part of our culture and it is how we all learn and make ourselves better. We even review code written by our president. Jason’s most recent code review had 95 issues with over 70 issues that were required to be fixed. To be fair, he wasn’t current with our latest standards when he took on that project, but he sure is now!
Review Your Own Code
Lastly, as the initial author, always do the first pass on your own code review. Hopefully, you will catch anything that is obvious and can fix it before others put their time on it. Also, if there is confusing code, or a section that you know will get lots of comments, add a note from the author as to why you did it the way you did.
Meet the Author: Jonathan Trick
Since he’s one of several “Jons” (or “Johns”) in the office, we call him by his last name, Trick. Which, of course, leads to seriously bad puns, like “That’s a Trick question!” or “That was a Trick-y one!” As our Director of Software Delivery, Trick is the one that makes sure RoboSource software is written correctly and getting done on time. Since we are able to boast 96% on-time delivery, you know this guy is doing his job!
Recent Comments