Are You Still Making These Common Code Review Mistakes?

Hey, fellow coders! Let's talk about how we review code. It’s an area where engineers’ soft skills come into their own. And for many, doing good code reviews is harder than writing good code.

Code reviews are an important process for the MadAppGang team, and I want to share some thoughts on how to make them even more valuable.

You’ve probably seen poor-quality feedback such as:

“We don’t do this kind of thing”

“Check the styleguide” and

“This can be improved”

Not only is this kind of language indirect and unhelpful, but it also creates unnecessary tensions. And it doesn’t help the person who created the code make any worthwhile improvements.

Instead of justifying and explaining their recommendations, pull request (PR) writers may try to pressure their colleagues, creating the potential for conflict. 

It should be remembered that PRs represent the efforts and creativity of specific developers. They put their time and energy into creating the code, and that deserves the best, most constructive feedback.

So what should you do to improve feedback and make the PR review process more productive and friendly?

1. Justify your recommendations

Instead of saying "You've made a mistake here, correct it with my example. Here's the link", try to justify your position. For example: “We have experienced a similar situation before. Here is how it was solved here → link. Can we implement something similar?”

Your colleagues can better understand your logic and discuss the pros and cons of your proposed changes.

Remember: your opinion on a piece of code must be supported by strong arguments. For example, better performance, more resilience to bugs, or a much simpler implementation that still preserves all the functionality. 

Arguments like 'my code is nicer' are not strong arguments.

2. Share context

Sometimes solutions that seem obvious to the PR author may not be obvious to other team members, especially new contributors. That's why it's important to share the context and explain how and when the decisions behind the proposed changes were made.

Example: "We discussed whether to go for async functions or stick with callbacks when dealing with asynchronous tasks. After weighing the pros and cons, we decided to go with async functions - they just give a cleaner and more straightforward feel. You can read the details of our decision in the guidelines [link]”.

3. Use static analysis tools

If possible, set up a code linting system that automatically checks code for compliance with standards. If this is built into the continuous integration (CI) process, the PR writer will get automatic feedback and often won't need to discuss coding style.

Applying linting rules also promotes code base consistency and simplifies the review process.

There is a fairly well-known guideline for the layout of commits – conventional commits. But there is also a lesser-known guideline for code review comments – Conventional comments. It’s a very ‘ecological’ way of code reviewing.

Conventional comments solve the problem of misunderstanding and even prevent conflict by adding labels and attributes to comments. Service offers a format where the message is described as:

<label> [decorations]: <subject>

[discussion]

• label – comment type
• subject – comment’s main idea
• decorations (optional) – additional labels for comment. They’re surrounded by parentheses
• discussion (optional) – it provides supporting statements, context, reasoning, and everything else to help communicate the "why" and "next steps" to resolve the comment.

For example:

question (non-blocking): At this point does it matter which thread wins? Maybe to prevent a race condition we should keep looping until they all win?

4. Pay more attention to the process of interaction inside your team

A crisis in architectural decisions indicates poor team organisation. The solution has already been implemented, so why are we only now realising that we have taken a wrong turn?

Communication style is important when reviewing PRs, especially in asynchronous working environments. At MadAppGang, we’re proud of our diverse and multinational team. With team members spread across different time zones, asynchronous communication is crucial. 

Your comments convey not only your ideas but also your attitude towards your colleagues, whether intentional or not. 

Friendly, persuasive feedback is not only more effective but also creates a positive working environment and builds trust. Spend more time on team building. It’s easier to have hard discussions among friends.

PR is an important part of software code development, and effective feedback plays a key role in this. 

Use direct and constructive comments, justify your recommendations, and share context to make the process more productive and friendly. Seek to persuade by showing why your suggestion is a good one. 

With friendly and constructive comms, it’s much easier to collaborate and achieve better results when developing code.

P.S. By the way, we have some useful tips on how to beat procrastination.