6、处理代码评审过程产生的抵触问题
Sometimes a developer will push back on a code review. Either they will disagree with your suggestion or they will complain that you are being too strict in general.
有时开发人员可能会对某次代码审查结果产生抵触。他要么就是不同意你的建议,要么就是抱怨你太严格了。
Who is right?
谁是对的?
When a developer disagrees with your suggestion, first take a moment to consider if they are correct. Often, they are closer to the code than you are, and so they might really have a better insight about certain aspects of it. Does their argument make sense? Does it make sense from a code health perspective? If so, let them know that they are right and let the issue drop.
当开发人员不同意你的建议时,可以先花点时间考虑下他们是不是对的。通常,他们比你更接近代码,因此他们可能会对代码的某些方面有更好的了解。他们的异议是否有意义?从代码质量的角度来看这些争议是否有意义?如果是的话,那就让他们知道他们是对的,然后关闭问题。
However, developers are not always right. In this case the reviewer should further explain why they believe that their suggestion is correct. A good explanation demonstrates both an understanding of the developer’s reply, and additional information about why the change is being requested.
然而,开发人员并不总是对的。在这种情况下,审查人员应该进一步解释为什么相信他们的建议是对的。一个好的解释不仅表明了对开发人员的回复的理解,也深入阐述了为什么要这么改。
In particular, when the reviewer believes their suggestion will improve code health, they should continue to advocate for the change, if they believe the resulting code quality improvement justifies the additional work requested. Improving code health tends to happen in small steps.
尤其是,当审查人员认为他们的建议会改善代码质量并且觉得由此带来的额外工作量是合理时,他们应该继续倡导修改。改善代码质量总是在小步中进行的。
Sometimes it takes a few rounds of explaining a suggestion before it really sinks in. Just make sure to always stay polite and let the developer know that you hear what they’re saying, you just don’t agree.
有时需要来回对建议解释个几轮才能让开发人员真正理解。在这过程中,也要记得保证礼貌并且让开发人员知道你知道他们在说什么,只是不同意而已
Upsetting Developers
Reviewers sometimes believe that the developer will be upset if the reviewer insists on an improvement. Sometimes developers do become upset, but it is usually brief and they become very thankful later that you helped them improve the quality of their code. Usually, if you are polite in your comments, developers actually don’t become upset at all, and the worry is just in the reviewer’s mind. Upsets are usually more about the way comments are written than about the reviewer’s insistence on code quality.
审查人员有时会觉得如果审查人员坚持代码改进会让开发人员感到沮丧。开发人员有时也确实会感到沮丧,但这种感觉通常是短暂的,并且后续还会感激你帮助他们改善代码质量。通常情况下,如果你在评注里保持着礼貌,开发人员实际上是不会感到抗拒的,这个担心只是审查人员自身觉得的而已。比起审查人员坚持代码改善,反而是审查意见的编写方式更容易让开发人员感到不快。
Cleaning It Up Later
后续优化
A common source of push back is that developers (understandably) want to get things done. They don’t want to go through another round of review just to get this CL in. So they say they will clean something up in a later CL, and thus you should LGTM this CL now. Some developers are very good about this, and will immediately write a follow-up CL that fixes the issue. However, experience shows that as more time passes after a developer writes the original CL, the less likely this clean up is to happen. In fact, usually unless the developer does the clean up immediately after the present CL, it never happens. This isn’t because developers are irresponsible, but because they have a lot of work to do and the cleanup gets lost or forgotten in the press of other work. Thus, it is usually best to insist that the developer clean up their CL now, before the code is in the codebase and “done.” Letting people “clean things up later” is a common way for codebases to degenerate.
发生抵触的一个常见原因是开发人员想完成任务,这是可以理解的。他们不想为了让这个CL通过而进行新一轮的审查。因此他们会说他们会在后续的CL里进行优化,现在你应该先让我通过这个CL。一些开发人员很擅长做这个,还会立即写一个跟进事项。然而,经验表明,开发人员在写下原本的CL之后,随着时间推移,这些跟进事项会越来越难以得到实施。实际上多数情况是,除非开发人员立马处理这些问题,否则以后都不会处理。这不是因为开发人员没有责任心,而是因为他们有很多工作要做,从而这些跟进事项在其它工作的压力下被搁置或者遗忘。因此,最好坚持在代码入库前就让开发人员就处理好这些问题。让人们“稍后跟进”是代码质量下降的常见原因。
If a CL introduces new complexity, it must be cleaned up before submission unless it is an emergency. If the CL exposes surrounding problems and they can’t be addressed right now, the developer should file a bug for the cleanup and assign it to themselves so that it doesn’t get lost. They can optionally also write a TODO comment in the code that references the filed bug.
如果一个CL引入了新的复杂性,那么在提交到代码库前必须进行优化除非这是个紧急任务。如果这个CL暴露了一些别的问题而且无法立马解决,那么开发人员应该提交一个待跟进的bug并把它指派给自己,以此来让它不会被遗忘。开发人员也可以选择在代码里下一个TODO注释来提醒这个被记录下来的bug。
General Complaints About Strictness
If you previously had fairly lax code reviews and you switch to having strict reviews, some developers will complain very loudly. Improving the speed of your code reviews usually causes these complaints to fade away.
如果你之前做代码审核比较宽松,然后(突然)转向严格的审查,一些开发人员会可能意见会比较大。提高你的代码审查速度通常能让这些抱怨消失。
Sometimes it can take months for these complaints to fade away, but eventually developers tend to see the value of strict code reviews as they see what great code they help generate. Sometimes the loudest protesters even become your strongest supporters once something happens that causes them to really see the value you’re adding by being strict.
有时需要花几个月的时间来让这些抵触情绪消失,但最终开发人员会发现严格代码审查所带来的价值,因为他们看到了在审查人员的帮助下写出了出色的代码。一旦意见最大的那几个抗议者看到了你严格审查产生的价值,有时他们甚至会成为你的最坚定的支持者。
Resolving Conflicts
解决冲突
If you are following all of the above but you still encounter a conflict between yourself and a developer that can’t be resolved, see The Standard of Code Review for guidelines and principles that can help resolve the conflict.
如果你遵从了上述几点但仍然和开发人员产生了难以解决的意见冲突,那么请参阅代码审查标准章节,里面的指南和原则有助于解决冲突。