Development
6 May 2024
Talon.One Writer
Content Team
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:
Create a branch off master
Work (fix the bug, build the feature, refactor the legacy code, etc.)
Merge the branch back into master
But between point 2 and point 3, any well functioning team will implement this point:
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.
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.
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:
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:
Typos or oversights (mistyped variable names, forgotten commented code)
Code style (not using camelCase for a variable, formatting errors)
Mistakes (forgot to return the value of a function, forgot to catch errors)
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 the usefulness of an abstraction, or requesting the creation of one
Proposing the use of a library instead of building a custom solution
Proposing a simpler, more performant solution
Raising concerns about the legibility of the code
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:
Asking for a functional approach instead of an object-oriented one
Asking for a different set of core abstractions (components, classes)
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:
Shining light on some legacy code that should be refactored
Bringing to attention an unrelated bug discovered while reviewing
Talking about processes and workflows to be improved
Proposing a subsequent feature that this pull request enables
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 :
Use of one library over another
Naming of variables when legibility issues are raised
Use of one language feature over another
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.
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!
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:
✖️ Objective issue
🤔 Expressing concern or asking a question
❌ Questioning architectural choice
📎 Raising an issue unrelated to the current pull request
🤷♀ Nitpicking
💯 Complimenting the author
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).
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.
Making the reviewer feel seen
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:
A happy resolution
It is then the responsibility of the original commenter to either resolve the issue, or to follow through with further comments.
✅Merged
In the end, this approach tries to solve issues I have encountered many times:
Clearly defining the nature of an issue
Smoothly communicating issues in the code review process
Making the process easier and more fun!
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. 😃👍
Join thousands of marketers and developers getting the latest loyalty & promotion insights from Talon.One. Every month, you’ll receive:
Loyalty and promotion tips
Industry insights from leading brands
Case studies and best practices
Isabelle Watson
Loyalty & promotion expert at Talon.One
The World's Most Powerful Promotion Engine
BERLIN
Wiener Strasse 10
10999 Berlin
Germany
BIRMINGHAM
41 Church Street
B3 2RT Birmingham
United Kingdom
BOSTON
One Boston Place, Suite 2600
02108 Boston, MA
United States
SINGAPORE
1 Scotts Road, #21-10 Shaw Centre
228208 Singapore
Singapore
Product
Company