Cooking Effective Code Reviews.

Posted in Engineering on Programming, Processes
Cooking Effective Code Reviews.

Reviewing code (PRs) is not an easy task, so in this post I will share tips and tricks on how your code reviews can better contribute to code quality, be more effective and increase team morale by following a structured and organized process. Let’s jump in!

“HAPPINESS is not something ready made. It comes from your OWN ACTIONS.”

Introduction

Reviewing code is not an easy task. Most of the time code reviews come in a format of Pull Requests (PRs from now on). We all have our own style and guidelines when addressing them, so in this post I will share my own tips in order to make them effective and valuable.

As a starting point I would like to bring up a couple of inspirational coding quotes, which I keep in mind when writing code:

“Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.” - John F. Woods

“Programs must be written for people to read, and only incidentally for machines to execute.” - Harold Abelson

“I’m not a great programmer; I’m just a good programmer with great habits.” - Kent Beck

Cooking Effective Code Reviews IT WORKS ON MY MACHINE…

Let’s get started by exploring a bunch of areas, which will help us to create structure and organization within our code reviews, plus things to pay attention to within the process.

Purpose and Importance

Before jumping into the process of reviewing code, we need to understand the whys behind code reviews and how they contribute to better software development.

Let’s enumerate them:

  • They ensure code quality: four eyes see more than two.
  • They act as documentation: they could be used to understand, learn and go back in time to check technical decision making.
  • They encourage collaboration and contribution: team work for the win.
  • They cultivate engineering culture: a great opportunity to ask questions, share expertise and suggest changes and fixes.

Cooking Effective Code Reviews Effective code reviews ensure code quality.

PR Size

The first and most important part of a code review is its PR size. It is key to provide a concise size in order to facilitate the review: keep it short and straight to the point.

As a rule of thumb and in my experience:

  • 200 lines of code are great.
  • 400 lines of code are fine and manageable.
  • More than 500 lines of code is where things start to become overwhelming.

Remember that effective code reviews are small and often and if you feel you are breaking this rule, try to break code down into tinier chunks.

In situations like renamings or refactors which involve a bigger changeset (due to coupling, legacy code or tech debt: we have already been there right?), you can specify in the PR description or even *pair with someone and commit the changes directly.

PR Description

A good PR description is key in order to rapidly acquire context on what needs to be reviewed. Descriptions and PR themselves should follow the Single Responsibility Principle: do one thing and do it well.

They should basically contain (as short as possible):

  • Ticket: Link to a ticket from the issue tracker you are using.
  • Purpose: What?
  • Reason: Why?
  • Implementation Details: How?
  • Testing: Quick explanation of the test cases.
  • Documentation: Link to the documentation if required.

Boy Scout Rule: I know it is tempting to refactor something out of scope of a PR and include it in it, but… try to not fall into this trap and be gentle with the person reviewing your code. Being strict in this sense is important: you can always create another small PR with this changeset.

PRO TIP: If you are using Github, you can create templates (for consistency) with your desired fields to include within the PR description. This template will be triggered every time you create a PR.. Here is an example

Attitude and Communication

Communication in PRs is by nature asynchronous and because of this fact, we need to be careful to not block people. Reviewing code is a responsibility, it is not a “touch and go” process, so let’s ensure that we monitor our conversations and/or requested changes.

When it comes to attitude my tips are:

  • Always provide constructive feedback: Positive comments.
  • Remain objective: try to remove personal taste and always have technical reasons that back up your technical arguments.
  • Do not take any comment personally.
  • Impersonate comments and discussions: “Could you rename this constant?” Should be “this constant needs to be renamed”.

Cooking Effective Code Reviews We should always have a positive attitude and provide constructive feedback.

Code Quality

In the beginning of the article we have highlighted code quality as one of the strongest benefits of reviewing code, which rises the questions of: What do we need to pay attention to at this level?

Tests

The first thing I do when reviewing code is to scroll down to the bottom of the PR in order to see the existence of tests:

  • if I do not find any, then I will automatically request changes.
  • If I do find them, then I make sure:
    • I understand them, which helps to better understand the scope and purpose of the PR (tests by nature act as documentation).
    • They are well designed.

PRO TIP: There is existent tooling to measure code coverage overall in your codebase or per PR, like codecov:

Cooking Effective Code Reviews It is very important to include code coverage within PRs.

Design

To detect issues at code level, even though it is key to be proficient with the technology we are evaluating, it is even more important to be familiar with code anti-patterns, code smells, software engineering design principle and best practices, thus, we have this knowledge which will provide us with extra tools to easily detect potential issues.

In this aspect, I check:

  • Code smells: God classes/functions, magic numbers, naming conventions, code intention, etc.
  • Code is well-designed and consistent with the rest of the codebase.
  • There is no unnecessary complexity.
  • The functionality is friendly for the rest of developers to use/read/understand.
  • Changes look good If there is UI involved.
  • Parallel programming, security and any other aspect that belong to the scope of the PR.
  • Code Style: indentation, spaces/tabs/lines or any other aspect that conforms to the style guidelines we have.
  • Code documentation: for example an api or open source project.


Cooking Effective Code Reviews Effective Code Reviews contribute to increase code quality.

Tooling

  • Your IDE: The best way is to actually pull your PR branch and run it locally.
  • Your Git repository hosting service: For example Github or Gitlab, they offer friendly web tooling to compare code, comment, highlight, track changes, commit, etc.
  • If you manage your own Git Service, then Gerrit is a well known open source alternative developed by Google.
  • A couple of paid tools: Crucible or Upsource.

Conclusion

Code reviews done wrong could become a big evil, leading to critical issues, not only at a technical level but also around collaboration and team morale. If we create consistency, structure and organization when reviewing our code, we will be contributing to better processes, discussions and higher software quality.

There’s a lot to gain out of conducting effective code reviews and I hope this post has shaded some light on the topic. As usual, any feedback or tips are more than welcome! Happy Coding!