Development
-
Nov 25, 2019

A Proposal for Clearer Code Review

Mathieu Anderson
Frontend Developer
7
Min to read

Source control is a collaboration tool

In today’s software development processes, Git is the de-facto standard for source control. Everything revolves around the project’s repository, and whether your team uses GitHub, Gitlab or any of the other providers, software developers will (hopefully) always rely on the following workflow:

  1. Create a branch off master
  2. Work (fix the bug, build the feature, refactor the legacy code, etc.)
  3. Merge the branch back into master

But between point 2 and point 3, any well functioning team will implement this point:

2.5. Code review

Code review is the process in which fellow team members will read your code, assess its quality, and determine whether it is ready to be added to the codebase. It is absolutely vital, as writing software is a team effort, and code review is an important point of collaboration.

This can sometimes be a difficult process, and my article addresses one of the issues I have most commonly faced: how to make sure that the key intent and importance of a piece of feedback is well understood, whether you’re giving or receiving it.

Prioritizing comments is hard

You’ve just sent a pull request. It was a big feature, and there’s a lot of code to read. You’re proud of your hard work and are eagerly expecting the feedback from your teammates. So when it comes, it’s overwhelming.

So many comments

You get to work and start to read through all the comments. Some of them are trivial (typos, code style, personal preference), some of them are more serious (security concerns, performance, legibility), and some of them are directly questioning key architectural concepts. And then others that only raise questions that are not strictly related to the scope of the pull request.

You try to answer all of them, but it is hard to keep the focus on what is truly important when you’re following so many issues at once. And it is even harder to keep your teammates informed about the progress you are making. They might ping you to ask about it. Eventually, everything is resolved, and the pull request is merged, but you can’t help but wonder about the time lost by constantly switching context, communicating continuously throughout the process, and how you could do a better job of prioritizing issues next time.

Levels of concern

Not all feedback is equal. Not all issues raised need the same amount of attention. And this can be hard to assess, both when giving and receiving feedback. The first step is therefore to define categories. From personal experience and conversations with fellow developers, all comments on a pull request will probably fall into these broad categories:

Objective issues

These do not warrant conversation: they point out errors or a breach of internal standards. They should unequivocally be fixed, usually in the way suggested. They can be:

Expressing concern or asking questions

These usually open a conversation. The reviewer doesn’t understand why a specific problem has been solved in this way and might offer an alternative solution. They can be:

Questioning architectural choices

These are the comments you never want to see in a code review. They express concerns that should have been raised earlier in the development process, because they put the whole design into question. They might be:

Raising issues unrelated to the current pull request

These are very precious comments that should be taken to heart by the whole team. They might point out some issues or bugs previously unnoticed, but that came up during the review process. They are not strictly related to the current code, though, and should be addressed in a separate pull request. They can be:

Nitpicks

These should be kept to an absolute minimum on a pull request. They express personal preferences and general bikeshedding. They can be hard to resist as a reviewer… But they should not request changes. Implementation of these suggestions should be entirely up to the pull request’s author. They might include :

Complimenting the author

These are the comments you definitely want to see in a code review! They point out elegant code and clever solutions, approving the choices made and comforting the author in their skill. They are a very important part of any code review, and vital to healthy teamwork and morale.

As we can see, there is a very high diversity in both the scope and the nature of the comments that we can encounter in a code review. My objective is to make each type of issue immediately identifiable and to ensure efficient communication around it.

Proposal: emoji-based communication

Emojis have become a very significant part of digital communication over the years. In 2015, the Oxford Dictionary even announced 😂 as their Word of the year! They are ubiquitous in the collaboration tools commonly used by software development teams (GitHub and Slack at the forefront). And they are fun ✨So let’s use them!

Categorizing comments ✖️🤔 ❌ 📎 🤷‍♀ 💯

Using a defined list of emojis, we can help every team member to quickly identify the kind of issue a comment raises, as well as immediately clarify the intent of it. Using the previously defined categories, here’s what your list could look like:

A reviewer would always begin their comment with the appropriate emoji. Like this:

✖️ adminPassword instead of aminPassword.

🤔 Maybe we could use moment instead of implementing our own time parsing.

📎 It looks like this button is bugged in the loading state, I will create a ticket for it and look into the issue later.

🤷‍♀ I really prefer reduce to using filter and map together like this.

💯 I LOVE THIS CODE SO MUCH THANK YOU OMG!

This immediately helps with scanning the comment list and deciding which issue to tackle first, without having to actually read the body of the comments (which can be quite long).

Tagging an issue as “In progress” 👀

So, once we’ve decided on which issue to tackle, it can be useful to inform our reviewer that we have seen their comment, and are working on it. Here, the emoji reaction feature of GitHub helps us. We can simply add 👀 to the comment.

GitHub comment tagged with eyes emoji
Making the reviewer feel seen

Resolving issues 👍

Again, leveraging the emoji reaction feature, a well-placed 👍 lets everyone know that you have addressed the concern and pushed a fix. It should be accompanied by a comment linking to the relevant commit, and might look like this:

GitHub comment tagged with thumb up emoji and answered with link to commit
A happy resolution

It is then the responsibility of the original commenter to either resolve the issue, or to follow through with further comments.

A hopeful outcome

Merged commit into master
✅Merged

In the end, this approach tries to solve issues I have encountered many times:

My proposal of this approach to my colleagues was met with enthusiasm, and I am looking forward to seeing, what I expect to be really great results from it. 😃👍

free materials