I constantly make commits to open source projects (Red Hat, etc.). And he noticed that negative code reviews, which are essentially subjective, take the most time. Most often this happens with commits, where the maintainer for some reason does not like your change. In the best case, such a code review strategy results in wasted time in meaningless debates; in the worst case, it actively discourages committing, creating a hostile and elitist environment.

The code review should be objective, concise and, if possible, contain only certain facts. This is not a political or emotional debate, but a technical one. His goal is to move forward, develop the project and all participants. Any commit should be evaluated only on the merits, and not on a subjective opinion.

Code Review Strategies

Here are a few strategies to keep in mind when considering code that you (as a maintainer) for some reason don't like:

1. Rephrase the objection as a question

• Bad: “This change will make XXX impossible” (This is an exaggeration; is it really impossible?)
  
• Good: "How will we do XXX after your change?"

2. Avoid exaggeration

Simply express your concerns and ask questions to help achieve your desired outcome.

• Bad: "This change will destroy productivity."
  
• Good: “It seems that X may be slower than the existing Y; "did you take measurements / collect data to show that this is not so?"
  
• Better (if you have the time): “For now, I am collecting data. I’ll try to verify that X is not slower than Y. "
  
• Also good: “This change changes a single O (n) cycle to a double-nested O (n²) cycle; will it affect performance? ”

3. Leave malicious comments to yourself

Some thoughts are best kept with you. If you can’t say politely, better keep silent.

• Bad: “I think this is a bad change that will ruin everything.”
  
• Bad: “Are you sure software development is the right career choice for you?”

4. Act positively

Maybe you had a different idea on how to solve the problem? If you act positively, you will ultimately find a solution that is better than any of the original options.

• Bad: "This change sucks, my version is better."
  
• Good: "I also have a change for this place: perhaps we can compare and / or combine ideas."
  
• Also good. "I have a similar change in my work, but I decided to make X, because ZZZ; why did you choose Y? "

5. Remember that everyone has a different experience.

In all other respects, an absolutely competent engineer may not know for some years some facts that you take as common sense. There is nothing wrong with saying the obvious thing, unless you patronize or maliciously.

• Bad: “Can't you see that this is clearly wrong?”
  
• Good: "This is not true because it throws a null pointer exception when X is Y."

6. Don't downplay the complexity of what's not obvious

Remember that things that are obvious to you may not be obvious to everyone. By offering alternative approaches and pointing out useful examples, you can help everyone sync up.

• Bad: “Why not just crush the gnosis?”
  
• Good: "If you crush the gnosis, then this part can become easier (see XXX for an example)."

7. Respect

Sometimes a commit simply does not meet the minimum quality standard. It is normal to say this, but showing respect does not require additional effort.

• Bad: "This is stupid code written by a stupid person."
  
• Good: “Thank you for your input. However, it cannot be adopted as it stands; there are many problems (as indicated above). ”
  
• Also good: “As stated above, there are several issues with this commit. Maybe step back and talk about usage scenarios? This will help to find the way. "

8. Manage your expectations (and your time)

If the commit is too large and cannot be quickly considered, it’s normal to say so right away. Then look for a solution.

• Bad: "I do not accept it, it is too big."
  
• Too bad: ignore the commit until it disappears.
  
• Good: “Could you split it into smaller commits? I don’t have too much time for a code review, but it’s just too big / complicated commit for one review. ”

9. Say “please” (show courtesy)

Just saying “please”, you show that you respect the time of the sender, especially when you want to change the formatting or style, which may seem like a slight change. Examples:

• “Could you document the changes in the gaps in another pool request?”
  
• “Could you align these variable definitions to make them easier to read?”

10. Start a conversation

If after all this you still don’t like something, but you’re not sure why, you may just have to put up with it. Or say, “I don’t like it, but I’m not sure why, can we talk about it?” This is a reasonable question, and even though it may take a little time, it is often worth it because you now have two people, and both learn (explain and listen), not two people who are opposed to each other.

Even qualified and experienced engineers should be able to say, “I don’t understand why I don’t like it.” This is not an invitation to attack the position of the reviewer, but rather an honest search for knowledge.

Summary

Avoid hyperbolic or pompous statements, avoid disputes, elitist or degrading expressions and constructions like “obvious” and “why don't you just ...”. Use clear, factual statements and supporting language, ask questions and move forward. Remember that colleagues and contributors are people too. Their time deserves the same respect as yours.