The Code Runners cheat sheet for doing code reviews
Code review is part of the agile methodology and as such, our development teams at Code Runners strive to perfect it. It’s a useful practice and we often see a noticeable effect on application performance when conducting code reviews. There are several ways it can be done and we’ve previously written about the most common one — pair programming. But to be honest, pair programming is not something we can afford to do on a day-to-day basis. I doubt any company can. Practically, the most common way to do code reviews is by an “over-the-shoulder” approach or email pass-around.
The two are pretty much self-explanatory and the formal structure is not so important as the content of a review. This is what we try to focus on and through the years we’ve developed an internal checklist of things to remember while doing a code review. We split this up into mindset while doing review vs. mindset while being reviewed.
Doing a code review
Most people feel proud of the code they write. It may not be an algorithm for launching rockets, but if you’ve struggled with a feature for a few days and finally came to a solution, you’ll have a much deserved sense of accomplishment. Bear that in mind when you’re reviewing another person’s code. Be gentle, but also straight to the point and not condescending. You’re not a teacher, but merely a peer, who’s giving his opinion.
Understand the goal of the code. Think of the objective and identify, if possible, opportunities to simplify the code, while still maintaining the end solution.
Don’t give feedback just for the sake of feedback. If you do identify alternatives, assume that the author has already considered them and discuss together pros and cons to using them. Understand the author’s perspective.
Be mindful of the discussion staying on point and not turning too philosophical or academic. Nothing wrong with that, just move that type of discussion to a later time, after the review is finished.
When doing a review online and not in person, bear in mind that text cannot convey the full range of emotions. Something that sounds as a good-intended advice in your head might come off as an aggressive response in writing. Add clarification where necessary, ideally in person.
In the end, let the code author decide which alternative implementation to use.
Being Reviewed
First and foremost, forget about your pride and ego. Yes, you’ve written a 30-line masterpiece. Now accept that there might be other ways to implement that functionality and some of them might even be better. Don’t take the feedback personally. It’s not a review of your skills, but of the code you’ve produced.
If you feel that an online review is too aggressive, consider that the other side most likely doesn’t mean it that way. Talk to your colleague in person and clarify any misunderstandings before they turn into bitter feelings.
Explain as best as you can why you made the choices you made. It’s not so much a defence, but rather a justification of the solution you chose to implement. Help the reviewer understand your perspective, but also try to understand his/hers.
Finally, stay grateful for the provided feedback — after all the reviewer is not doing it for his own sake, but for everyone’s benefit. If you’re using a CI tool wait to merge until you get a green test suite and merge your changes only when you’re confident of your code’s impact on the application.
Both sides
There’s also several key points that should be considered no matter which side of the code review you’re standing. Many are quite straightforward and people would assume they go without saying, but they often get ignored in a review setting.
For example, don’t use sarcasm. It’s aggressive and doesn’t lead to any actionable effect, except to annoy the other side.
Instead of making demands, rather ask questions. It’s just a matter of how you structure your sentences, but it makes a huge impact — “Why don’t we name this class blabla?” vs. “Name this blabla!”.
Try to avoid using terms that convey ownership of certain parts of the project, such as “mine”, “your”, “hers”. It further adds a barrier between you and the other person and pushes you away from reaching a common ground. Also omit words such as “dumb” or “stupid” even though you may be colleagues and friends. Such adjectives can be associated with personal qualities, which again may create a distance between you and your peer.
In conclusion, code reviews can be a good opportunity not only to create a functioning, clean application, but also for developers to learn new approaches by understanding other points of view. The most important ingredient for code reviews to stay useful and not turn into a hate war is to accept that many code decisions are based on opinion. Pros and cons can be discussed in order for a common solution to be reached. Remember to keep an open mind and stay humble.
Thanks to Thoughtbot for sharing their code review guide and serving as inspiration to make our own one!
