代码审查经常引起争议。 在AlterConf会议上准备“我们从代码审阅中的有毒行为中 吸取教训 ”的演讲时，我准备听到很多反对意见和批评。 但是她没想到社区会如此支持这个想法。 我承担了抵抗，但是社区非常欢迎我并给予了我认可。

我被要求分享这些幻灯片 ，但现在我认为这些幻灯片本身没有什么用处，并且没有任何上下文：它们缺乏解释。 因此，我决定发布此文章。 后来，会议组织者上传了一个视频 。

下面列出了一些类型的非生产性代码审查行为以及有关如何使过程更友好并消除毒性的建议。 所有行为模型均已由我或在我认识的公司中进行过测试。 过去，这种罪过也带给了我。

非生产性行为1：事实交流

如果您不能参考文档，形式化建议和支持这些声明的代码示例，请不要发表声明。 人们应该知道为什么要求他们进行更改，而另一个开发人员的个人偏好还不够充分。

而不是说：

该组件应设置为无状态。

...最好提供一些背景信息：

由于此组件没有生命周期或状态方法，因此可以使其变为无状态。 这将提高代码性能和可读性。 *这里*一些文档。

通常在样式和语法的讨论中可以发现意见的转移是事实。 这些是非常重要的主题，但对于代码审阅而言却不是，因为样式和语法与开发人员最初解决的问题无关。

最好单独进行此类讨论并确定整个团队的风格。 实施linter和自动代码修复 。 然后，您将参考这些样式建议，而不是您的个人意见。

如果您在团队或公司中拥有较高的职位和权限，那么不要发表自己的观点，这一点尤其重要。 开发人员的印象是他别无选择，只能服从您的要求。

非生产性行为＃2：评论雪崩

当一个人犯了一个错误时，这个错误很可能在多个地方发生。 我注意到，审阅者有时会指出每一个案例，而不是留下详细说明和指向有用资源的链接。

注释的合并使您可以发送相同的消息，而无需抑制作者。 在一个问题上出现大量重复评论的现象似乎很挑剔。

无效和压倒性的：



一个重复发生的错误有多个注释（行末尾有空格）

一个更有用的选项：


综合反馈

我了解有时指向错误的每个位置很有用，因为在后续的提交中更正后注释会消失。 但是，如果错误多次发生，则很明显，开发人员没有意识到具体的指南，他应该指出正确的资源。

非生产性行为3：要求工程师“在他们在这里时”解决其他人的问题

不要要求开发人员处理与变更集不直接相关的问题或此代码试图解决的问题。 即使开发人员以大量不良做法扩展或修改了肮脏的代码，也不要要求他清除并修复此奇怪的代码。

我不建议仅仅因为开发人员最初没有编写代码就剥夺他们的责任。 实际上，说“别人的代码不是我的问题”之类的话是不好的。 仅创建一个单独的票证并拉出请求以修复脏代码更为合适。 因此，您避免了某人的工作量急剧增加，从而解决了技术债务问题。

TL; DR ：请勿要求开发人员“在问题出现时”解决问题。 如果该代码满足票证中的要求，并且没有在代码库中引入新问题，请批准它，然后创建票证以清除代码库。

非生产性行为4：提出谴责问题

避免问诸如此类的谴责问题：

你为什么不只在这里做____？

这意味着一些简单的解决方案应该是显而易见的。 这迫使开发商捍卫自己。

通常，这些谴责性问题只是面纱。 取而代之的是，提供建议（包括文档和链接），而忽略苛刻的用词。

您可以执行____，这具有____的优势。

非生产性行为5：讽刺

评论中没有讽刺的地方。 通常，讽刺的评论不会提供上下文和有效的反馈。 取而代之的是，详细描述问题并提供建议，但不要再开玩笑了。

无效：

提交之前，您是否还测试过此代码？

有用的

未能输入负数。 你能这样做吗？

这是代码复习中的另一个示例注释 ，它既不有趣也不有用：

我们不是邪恶的，我们是无情的。 如您所见，我写了“哔！” -导入您触摸的每个文件时。 我想到了以下几点：“您的导入违反了我们的标准约定，该约定以一定的顺序为先：首先是内置模块，然后是第三方，然后是项目级别，”但是此短语太长，无法在每种情况下键入。

在上面的示例中，“哔！” -完全没有用，也没有意义。 这是腐的幽默，对作者没有帮助。

非生产性行为6：表情符号，而不是描述问题

指出代码中的问题时，请避免将拇指朝下的表情符号或生病的小矮人的表情符号。 出于同样的原因，它和讽刺一样没用。 表情符号很神秘，很容易造成误解。 人们在浪费时间试图理解您的想法。


不要使用表情符号来表示编码问题

无论如何，您都不应对编程错误有任何情感上的反应。

使用积极的表情符号（如“竖起大拇指”或“欢呼！”）是很正常的，但不是要指出问题，而是要对好的代码做出回应。


批准表情符号很棒

非生产性行为7：不要回应所有评论

作者也为讨论的毒性做出了贡献。 如果您在不回答所有注释的情况下组合了代码，那么对于此人而言，这是令人惊讶的：他试图为您提供帮助，并且您明确表示某些评论无关紧要。

如果评论无关紧要，或者您将不采取任何措施，请简要说明原因。

不要忽略同事。


组合代码而不回应评论

非生产性行为＃8：如果来自顶级专业人员，则忽略有毒行为

不应仅仅因为开发人员是优秀的专业人士和生产性很高的雇员就忽略或低估毒性行为。 尽管他的工作出色，但请务必记住，他的不良行为会导致其他开发人员感到压力和精疲力尽。

与表现出中毒行为的人的经验：

其他人会发现与这个人一起工作会耗尽和消极。 他们会尽一切努力避免与他互动，即使这会对他们执行任务的能力产生负面影响。 整个组织内的通信将被中断，员工将开始在其他地方寻找工作。 当您面临人才流失和项目失败的后果时，这位特定的开发人员将继续冷静地工作，好像什么都没发生。 - 约瑟夫·杰弗罗

如果不采取措施纠正这种情况，则可能会造成严重后果：开发商将负担沉重，受到攻击并失去动力。 他们将开始害怕反馈，而反馈实际上将帮助他们成长。

每当我收到一封前同事对我的请求提出评论的电子邮件时，我个人都会感到非常担忧（以他的有毒无助的评论而著称）。 尽管毒性行为对每个人的影响都不同，但绝对没有人会从这种毒性中受益。

注意 ：我想说明的是，如果开发人员无法抵抗并且一旦表现出这种行为，就不会使他“有毒”。 我们正在谈论反复的侮辱和苛刻的评论。

有用的代码审查实践

以下是适用于任何常规通信的一些建议，尽管在此我们以编程和代码审查为背景。

有用的行为1：使用问题或建议来建立对话

切勿要求他人进行更正或提出不赞成的问题，因为这会干扰您与他人之间的对话。

您为什么不将这些转换与常量一起合并在文件中？

形式上，这是一个问题，但不会在您和对话者之间建立对话。 他只是让他为自己辩护。 而是问其他人对您的决定有何看法，例如：

您如何看待将这些转换转换成常量文件？ 它们很多，因此，现在单独使用一个文件可能很有意义。

...或提出建议：

在此文件中，您对“函数x”有很多翻译要求。 用其常量创建一个单独的文件可能很有意义。

有用的行为＃2：协作，不要躲藏

当您一起编程时，您必须周围，提出问题，讨论并指向资源。

“……当您想提供帮助或一起工作时，必须充分参与，而不仅仅是有时进行干预”-《递归中心指南》

有用的行为＃3：回应每个评论

如果您作为作者不打算应用审阅者的建议，请通过报告进行注释。 不要忽略那些花时间帮助您的人。

例如：

人员A：-您如何为该API调用创建帮助函数？ 否则，一切都很好。

人B：-这行不是我的变更集的一部分。 我将合并代码，在GitHub上创建一个单独的票证，并将其写到我们小组的工作计划中。

有用的行为4：知道何时安排面对面的会议

经过数十次评论和提出的解决方案后，应该清楚的是，在线交流对于当前问题已变得毫无用处。 向所有参与的同事发送会议邀请。

因此，该小组将能够迅速做出决定并加以应用。

花费数小时和大量评论的问题通常可以在几分钟的富有成效的对话中解决。 -“整洁的Java”

有用的行为5：利用学习机会，不要炫耀

在参加代码审查之前，请问自己：

您的评论是否真的有助于您学习或发现错误？

考虑一下您的评论。 请记住，代码审查的目的是教导和帮助其他开发人员成长。 这不是表演平台。

有用的行为6：不要表现出惊讶

如果一个人不知道某事，请小心不要表现出惊讶。 不要让人们认为他们应该已经知道此信息而感到不安。 相反，请随时承认您缺乏有关该主题的经验-这是寻求帮助的好方法。

有关更多信息，请参见递归中心教程中的“不要假装感到惊讶”规则。

有用的行为7：自动化一切

讨论可以被棉短绒，Git钩子或自动测试捕获的错误几乎没有意义。 这似乎是挑剔的大量额外评论。 人们不太擅长识别此类错误，因此已经创建了自动化工具。

还有一些工具可以在添加新代码时自动运行测试。 如果一组更改违反其中一个测试，它们将显示警告。 例如，TeamCity和Jenkins CI。

另外，使用Git挂钩对新代码运行测试和查询。 如果发现错误，他们将拦截提交。

让该工具指出问题，使人们不必这样做。

有用的行为＃8：不要超过有毒行为

您的代码审查中无需采取有害行为，因为对于新开发人员而言，这看起来像报仇或某些令人讨厌的仪式。

寻找可以支持您的同事。

如果您发现无效的代码审查，请尝试告诉同事，坦率地说（如果您觉得自己的位置/公司安全）。

实用行为9：管理者，仔细考虑候选人，听取团队意见并运用您的权威

经理们有很大的机会在团队中营造积极向上的氛围。

我们从“有害的有毒开发商”一文中改写提示：

• 防止有毒的开发人员出现在团队中。 招聘时，不仅要注意技术培训。 了解候选人如何进行协作和交流。 批判地分析他的工作，看看他的反应。 确保每个候选人都可以改善公司的文化，而不仅仅是使其适应文化。
  
• 当有毒的开发人员仍然进入州或继承州时，请在个人交谈中向每位员工征求意见。 如果开发人员有毒，则会在有关他的评论中弹出。
  
• 与有毒的开发人员交谈。 在代码审查中给出有效注释的具体示例。 不断与他合作，继续与同事和经理进行个人对话来收集信息。
  
• 不要隔离有毒的显影剂。 （*）
  
• 向员工重复有关友好环境的需求。

（*）尽管文章建议隔离有毒的开发人员，但我认为鼓励有毒的开发人员与团队合作（但以更健康的方式）很重要。 绝缘不能解决问题。 如果您单独进行工作，则短期内毒性会降低，但开发人员不会放弃其非生产性行为。 他只会失去登上领奖台来证明这一点。

有用的行为10：在团队规模较小且不断壮大的同时制定标准

小型团队能够接受并实施新想法。 即使您认为没有必要设定标准，因为现在已经没有问题了，但是重要的是在补货到来时保持良好的合作惯例。

有用的行为＃11：了解您可能会成为问题的一部分

为了创造一个更有利的环境，对自己诚实并反思任何无效的行为非常重要。

大三时，我几次注意到某人的代码中有一个错误，并且很高兴，因为我可以发表评论！ 现在我知道我利用这次机会吹牛，而不是帮助。 您需要仔细评估自己的行动。

我认为对每个人来说，花些时间进行这一艰难的内省是很有用的。

最后一个...

我们不是在谈论评论的内容，而只是在谈论基调

我知道审核很重要，我们并不反对这些审核。 我不要求您牺牲代码的质量。 仁慈的代码审查文化和高质量的代码不应相互排斥。

我只是希望人们能够采取步骤提供建设性的，有效的反馈，并创造更有利的条件，使开发人员感到自在，学习，成长并且不怕犯错误。 我们都可以一起进步。