Code Review: Easier Better Faster Stronger

Yes, you read it right. I took the liberty to adapt Daft Punk’s song title to talk about code review. As I write this, I’m wondering  if it is going to pass the thorough examination of the chief editor, but I like how the title sounds (and it really describes how a Pull Request should be). And you see, even this harmless piece of text is going over a rvesoin revision process before you can have the chance to be struck by my insights, so why shouldn’t we do the same with our code?

Code review is the process of having a peer developer reading your code after it’s done or during development time (pair-programming). The main goals are to find overlooked mistakes, guarantee the maintenance of the code style guidelines and discover more elegant ways to solve the same problem, taking into account scalability and performance, for instance. The result is software with better quality and wider involvement of the development team. Having your code inspected by someone else has many positive impacts:

1. Find errors, possible vulnerabilities and under-performing code before it reaches the final user (or even before it reaches QA, depending on your process).
   
2. Spread knowledge among all participants, making sure there are at least two people that know what’s going on with that chunk of code.
   
3. Encourage constructive discussion with other developers: when people dive deeper into the code as a group, they tend to write better code as a group.
4. Distribute responsibility and ownership. This way, nobody has to feel the pressure of failure on their own and everybody takes credit when it succeeds.
5. It’s an excellent way for junior developers to learn with more seasoned professionals.

Despite all these advantages, a lot of developers still shiver when they are assigned to a Pull Request. Code review is many times taken as unnecessary, inefficient and deadly boring. Don’t get me wrong: I understand this is sometimes the case. What I want to point out is that the problem usually isn’t the code review itself, but the process involved. Picture the following scenario:

You see a new Pull Request assigned to you. At first, you have no idea what’s it all about. The title reads: “Improvements and refactor”. You click on the link and see an ocean of green and red marks: four thousand additions, a thousand deletions. You don’t freak out, start full gas: “comment it, scroll it, improve it, code it” (read with robotic voice). Soon you are bored, comments are less and less frequent, and the code review loses its purpose.

This is not because the revision process doesn’t work at all, it’s simply because it’s being done the wrong way. Here are some principles we try to follow at our best here at Cheesecake Labs to improve code review quality:

1. Keep your PR focused on its goal: if you just want to fix a typo, that’s all you’re going to do. Don’t take the chance to do small refactorings here and there, they are a big source of distraction for those reviewing your code.
2. Write meaningful commit messages and PR titles: give people some context on what are you trying to achieve with your code.
3. Try your best to keep it under 500 lines between additions and deletions: less lines of code means less time reviewing it (and a more concentrated reviewer as a result).
4. Use linting and automated tests: the reviewer shouldn’t waste time with what the computer could have done.
5. Keep related code as close as possible: if two files have related code – let’s say the view and the controller – save them close to each other, in the same directory if possible. This way they are going to appear one right after the other on the code diff.
6. Keep the trailing comma in multiline literals (arrays, objects, dictionaries): it may sound small, but it can save a considerable amount of time with merge conflicts. As a plus, it appears as a one-line change on versioning diff, instead of two.
7. Follow a code style pattern: our brain is trained to understand patterns. If you always structure imports, docstrings, breaks and naming the same way, your code will be less tiring and easier to read.
8. Last but not least: don’t take things personally. Unless someone is really rude with comments, everything is annotated to improve the product’s overall quality. You can still be friends and have a beer together later.

Here at Cheesecake Labs we take code review as a core part of our culture. We use it to have awesome conversations among devs, learn new technologies with more experienced peers, let others know how we solve problems and most importantly: deliver great software as a team.

By the way, want to learn Pull Request submission best practices? Check out this post that my friend Iacami wrote about it: Git Etiquette: Submitting a Pull Request.

Cover Image: Adapted from “Daft Punk by Shaun Laakso”. Creative Commons Attribution-NonCommercial 4.0