评审你不喜欢的代码的 10 个友情提示 未翻译

oschina 投递于 2019/07/10 18:19 (共 10 段)
阅读 1898
收藏 1

As a frequent contributor to open source projects (both within and beyond Red Hat), I find one of the most common time-wasters is dealing with code reviews of my submitted code that are negative or obstructive and yet essentially subjective or argumentative in nature. I see this most often when submitting to projects where the maintainer doesn’t like the change, for whatever reason. In the best case, this kind of code review strategy can lead to time wasted in pointless debates; at worst, it actively discourages contribution and diversity in a project and creates an environment that is hostile and elitist.

A code review should be objective and concise and should deal in certainties whenever possible. It’s not a political or emotional argument; it’s a technical one, and the goal should always be to move forward and elevate the project and its participants.  A change submission should always be evaluated on the merits of the submission, not on one’s opinion of the submitter.

Code review strategies

Here are several strategies to keep in mind when reviewing submissions that, for whatever reason, you (as a project maintainer) do not like:

1. Rephrase your objection as a question

  • Bad: “This change will make XXX impossible.” (This is hyperbole; is it reallyimpossible?)
  • Good: “How can we do XXX with your change?”

2. Avoid hyperbole

Simply state your concerns and ask questions to help get to the desired outcome.

  • Bad: “This change will destroy performance.”
  • Good: “It seems like doing X might be slower than existing Y; have you measured/gathered data to show it isn’t?”
  • Better (if you have time): “In the meantime, I am gathering data to try to verify that X is not slower than Y.”
  • Also good: “This change changes this single loop O(n) to a doubly nested loop O(n²); won’t this affect performance?”
已有 12 人翻译此段(待审批)

3. Keep snide comments to yourself

Some thoughts are better kept to yourself. If you can’t be civil, don’t engage.

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

4. Engage positively

Maybe you had a different idea about how to solve a problem? If you engage positively, you might end up discovering a solution that is better than either original option.

  • Bad: “This change sucks, my version is better.”
  • Good: “I also have a similar change at this location XXX: maybe we can compare and/or combine ideas.”
  • Also good: “I have a similar change in progress, but I chose to do X because ZZZ; why did you choose Y?”
已有 9 人翻译此段(待审批)

5. Remember that not everybody’s experience is identical to yours

An otherwise completely competent engineer could go for years without knowing some fact that you take as common sense. It’s okay to state the obvious, as long as you aren’t patronizing or snide about it.

  • Bad: “Can’t you see that this is obviously wrong?”
  • Good: “This is incorrect because it causes a null pointer exception when X is Y.”

6. Don’t diminish the complexity of something that’s not obvious

Remember that things that are obvious to you may not be obvious to everyone. Suggesting alternative approaches and pointing out useful examples can help get everyone on the same page.

  • Bad: “Why not simply frob the gnozzle?”
  • Good: “It might be possible to frob the gnozzle, which would simplify this part (see XXX for an example).”
已有 8 人翻译此段(待审批)

7. Be respectful

Sometimes a submission just doesn’t meet a minimum standard for quality. It’s okay to say so, but it doesn’t cost anything extra to be respectful.

  • Bad: “This is stupid code written by a stupid person.”
  • Good: “Thanks for your contribution. However, it cannot be accepted in its current form; there are multiple problems (as outlined above).”
  • Also good: “As outlined above, there are multiple problems with this submission.  Maybe we could back up a step and talk about the use cases instead?  That could help us find a path forward.”
已有 5 人翻译此段(待审批)

8. Manage expectations (and your time)

If a submission is too large to be reasonably reviewed, it is okay to let the submitter know right away. Keep moving forward.

  • Bad: “I’m not merging this, it’s too big.”
  • Also bad: Ignoring it until it goes away.
  • Good: “Could you please break this down into smaller changes? I do not have a lot of time for code reviews and this one is just too large/complex to review in one pass.”
已有 5 人翻译此段(待审批)

9. Say please

Just saying “please” goes a long way toward showing that you respect the submitter’s time, especially when you want something to be different due to formatting or style, which might seem to be a minor detail of the change. Examples:

  • “Could you please separate the whitespace changes into another pull request?”
  • “Could you please align these variable definitions so they’re easier to read?”
已有 6 人翻译此段(待审批)

10. Start a conversation

If, after all this, you still don’t like something but you’re not sure why, you might have to just live with it. But it’s also okay to say, “I don’t like this and I’m not sure why, can we talk about it?” It’s a reasonable thing to ask, and even though it might take a little time, it’s often worth the investment because now you have two people who are both learning (one by explaining and one by listening) rather than two people who are opposed to each other.

Even skilled and experienced engineers should be able to say “I don’t understand why I don’t like this”; it’s not an invitation to attack the position of the reviewer but rather an honest quest for knowledge.

已有 6 人翻译此段(待审批)


Avoid hyperbolic or bombastic assertions, avoid argument strategies, avoid elitist or demeaning language, and avoid constructs like “obviously” and “why don’t you just…”.  Use clear, factual statements and supportive language, ask questions, and move things forward.  Remember that coworkers and contributors are human people, and their time is worthy of the same respect as yours.

已有 8 人翻译此段(待审批)
我们的翻译工作遵照 CC 协议,如果我们的工作有侵犯到您的权益,请及时联系我们。


作为开源项目(包括Red Hat内部和外部)的频繁贡献者,我发现最常见的浪费时间的行为之一是处理提交代码的代码评审,这些评审是负面的或阻碍的,但本质上是主观的或争论性的。当提交到维护者不喜欢更改的项目时,无论出于什么原因,我经常看到这种情况。在最好的情况下,这种代码评审策略可能导致在无意义的争论中浪费时间;在最坏的情况下,它会阻碍项目的贡献和多样性,并创建一个充满敌意和精英主义的环境。




1. 把你的反对意见重新表述为一个问题


好:“我们怎么用你的零钱做XXX ?”

2. 避免夸张





还好:“这个变化改变这单回路O (n)双重嵌套循环O (n²);这不会影响性能吗?”





4. 积极参与