制定更好的代码审查

根据 RailsConf 2017 上的演讲改编并重新制作。

人与科技的交汇从来都不是简单易行的。对于创造科技的人来说，这一点尤为明显。作为一个以编写代码为生的人，我在代码审查过程中对此感受最深。

大多数开发人员倾向于将代码视为一种技艺——就像艺术家和大多数各类创作者一样——我们对自己的代码产生了强烈的依恋。我们被告知要做一个无私的程序员，不仅要批评我们自己的代码，还要批评我们办公桌上等待合并到项目代码库的每一行代码。我们听说让同行审查自己的代码和审查同事的代码都是“非常好的事情”，我们都应该这样做。而且，我们中的很多人已经在做所有这些被强烈推荐的事情了。

但是，我们上次评估这些方法论是什么时候？我们当中有人确定我们的代码审查流程真的有效吗？我们确定它们发挥了它们最初的设想吗？

如果没有的话，我们该如何尝试让它们变得更好？

为什么还要费心去审查呢？

在我们真正理解同行代码审查的实际好处之前，了解它最初诞生的原因会很有帮助。目前已经有相当多关于代码审查最佳实践的研究，但我认为 Steve McConnell 在《Code Complete》（最初发表于 1993 年）上发表的研究是一个不错的起点。

在他的书中，他描述了代码审查及其应发挥的作用。他写道：

软件工程流程管理的一部分是在“价值最低”的阶段发现问题——也就是说，在投入最少、解决问题成本最低的阶段。为了实现这一目标，开发人员使用“质量门”，即定期测试或评审，以确定某一阶段的产品质量是否足以支持进入下一阶段。

麦康奈尔 (McConnell) 主张对每个团队进行代码审查，其最有力的方面是，他将其与他所说的“建设中的集体所有权”联系起来：所有代码都归一组贡献者所有，他们每个人都可以平等地访问、更改和修改集体拥有的项目。

代码审查的初衷是帮助我们在软件开发过程中拥有集体责任感。换句话说，我们每个人都是开发过程中的利益相关者，参与控制产品质量。

McConnell 继续强调了工程团队可以在日常工作流程中采用的几种不同类型的代码审查流程。我强烈建议您阅读《代码大全》，详细了解每种技术；不过，就我们的目的而言，一个总结就足够了。代码审查有三种格式：

1. 检查

检查是较长的代码审查，大约需要一个小时，其中包括一名主持人、代码作者和一名审阅者。

有效使用检查通常可以发现程序中约60% 的缺陷（无论是 bug 还是错误）。根据 McConnell 的研究，与非正式的审查实践相比，检查可以减少每 1000 行代码中 20% 到 30% 的缺陷。

2. 演练

演练是一场 30 到 60 分钟的工作会议，通常旨在为高级开发人员提供教学机会，向新程序员解释概念，同时也让初级工程师有机会推翻旧的方法。

排查有时非常有效，但其效果远不及更正式的代码审查流程（例如检查）。排查通常可以发现程序中20% 到 40% 的错误。

3. 简短的代码审查

正如其名称所暗示的那样，简短的审查速度要快得多；然而，它们仍然是对最容易出错的小变化（包括单行变化）的深入审查。

McConnell 的研究发现了有关较短代码审查的以下几点：

一家引入单行代码变更审查的组织发现，其错误率从审查前的55%下降到审查后的2%。80年代末，一家电信组织在引入代码变更审查之前，其代码正确率从86%上升到审查后的99.6%。

数据（至少是 McConnell收集的数据子集）似乎表明每个软件团队都应该进行这三种类型的代码审查。

然而，麦康奈尔的书最早是在1993年研究和撰写的。从那时起，我们的行业发生了变化，我们的同行评审方法也发生了变化。但我们今天的代码审查实施真的有效吗？我们如何将代码审查背后的理论付诸实践？

为了找到这些问题的答案，我做了任何有决心（但有点不确定从哪里开始）的开发人员都会做的事情：我询问了互联网！

好吧，好吧……”我向 Twitter 询问。

开发人员如何看待代码审查？

在深入探讨调查结果之前，先声明一下：我不是数据科学家。（我希望自己是，因为我或许能更好地分析这项调查的所有回复，甚至可能还能熟练地用 R 语言绘制图表！）。最终，这意味着我的数据集在很多方面受到限制，首先是因为它是一项 Twitter 上的自选调查，其次是调查本身预设了一个基于分支/拉取请求的团队。

好的，现在我们已经解决了这个问题：开发人员如何看待代码审查？

定量数据

首先，我们将尝试通过查看定量数据来回答这个问题。

首先，这个问题的答案很大程度上取决于你询问的是哪位开发者。截至我撰写本文时，我的调查已收到 500 多份回复。

参与调查的开发者主要使用 Java、Ruby 或 JavaScript 工作。以下是这些回复按开发者及其团队主要使用的语言进行细分。

我询问了每一位受访者在多大程度上同意这一说法：代码审查对我的团队有益。

总体而言，Swift 开发者认为代码审查对他们的团队最有益，平均得分为 9.5（满分 10 分，1 表示非常不同意），10 表示非常同意。Ruby 开发者紧随其后，平均得分约为 9.2。

虽然大多数受访者（约 70%）表示，每个拉取请求在合并之前都会经过团队内部人员的审核，但并非所有团队都是如此。约 50 位受访者（约占整个数据集的 10%）表示，只有当团队内部提出审核请求时，才会进行同行评审。

数据似乎表明，这种分布在很大程度上涵盖了各种语言和框架。在是否所有拉取请求都经过审核，或者是否需要先请求审核方面，似乎没有哪一种语言的结果有压倒性差异。换句话说，似乎不是语言或框架导致了更一致的代码审核文化，更有可能的是团队本身。

最后，对于那些在需要合并前审查拉取请求的团队中工作的开发人员来说，似乎大多数团队只需要另一个人在将代码合并到共享代码库之前进行同行评审。

定性数据

那么那些无法量化的东西呢？除了多项选择题外，调查还允许受访者填写自己的答案。而这恰恰证明了调查结果最具启发性，更不用说最实用了。

匿名回复中反复出现了几个总体主题。

最终，代码审查体验的成功或失败似乎取决于两个因素：审查过程中花费了多少精力，以及审查本身有多少实质内容。

如果代码审查投入的精力不够，或者内容缺乏实质内容，那么代码审查就很糟糕（会给审查者和被审查者留下不好的印象）。另一方面，如果代码审查过程很彻底，并且花费了大量时间以实质性的方式审查代码的各个方面，那么它总体上会给审查者和被审查者留下更积极的印象。

但我们所说的能量和物质到底是什么意思呢？

活力

判断代码审查投入精力的另一种方法是回答这个问题：谁在做审查？ 他们花了多少时间？

许多受访者正在进行代码审查，但许多人似乎对审查人员以及审查人员在审查或等待审查时所花费的时间感到不满。

以下只是该调查的部分匿名回复：

我们有个开发者，他盲目地点赞每个 PR，却很少留言。这个人是在试图钻“至少批准两次”的空子。这很容易看出来，因为他一分钟内就会突然批准 5 到 6 个 PR。

–

我发现第二或第三位审阅者在看到一次批准后往往更有可能批准。

–

有时候，根据 PR 提交者的不同，对同一段代码的审查也会有所不同。

–

团队中的每个人都应该得到平等的评价。我觉得资深员工得不到反馈的情况很常见，因为人们认为他们不会犯错，但他们完全有可能犯错，而且可能需要反馈。而初级员工则被吹毛求疵……记住，人们的自尊心可能会受到影响，而且他们很容易受到伤害。

–

提交过大，导致 PR 审核时间过长。人们不会在本地分支上进行测试。

–

特别长的 PR 需要更长的时间来审查，这是一个问题，因为它们对未来的分支/PR/合并影响最大。

–

当谈到在代码审查上花费（或未花费）多少精力时，最重要的结论可以归结为三件事 ：

1. 没有人会对仅仅流于形式且没有任何意义的代码审查流程感到满意。
2. 审查包含您不熟悉或没有上下文的代码的长 PR 并不是一件有趣的事情。
3. 人非圣贤，孰能无过。我们都是人，都应该接受评判，也应该公平地评价他人。

物质

代码审查的实质可以归结为一个问题的答案：在审查代码时，某人到底说了什么、做了什么或者让其他人有什么感觉？

与代码审查实质相关的回应大部分都基于人们在审查中所说的内容以及他们如何表达。

以下是调查中的一些匿名回复：

我会认真对待任何关于 PR 的反馈。如果把那个字符串改成符号能让你满意，那就改吧，然后继续。我不会试图为一个武断的决定辩解。这和在 IDE 环境中工作没什么不同，我的大脑很容易陷入“看到红色曲线，就修复它”的思维模式。我不在乎它为什么不高兴，只要它别再冲我吼就行了。

–

不要在评论中对宏观或架构进行评论。进行线下交流。这很容易让人陷入困境，造成挫败感。

–

我强烈地感觉到，当人们要求修改时，这很烦人，尤其是当他们不花时间解释这样做的原因，或者不留任何理由让他们犯错的时候。尤其是当他们只是在注释中重写你的代码，并告诉你要改成他们的版本时。

–

如果评论线程很长，则表明应该进行口头对话（在评论线程中报告共识）

–

人们需要更好地区分自己的风格偏好和那些对功能产生影响的反馈。对于初级员工来说，区分两者可能很困难。当多位资深员工给出相互矛盾的反馈时（例如，如何命名路线以及为什么），也会令人沮丧。

–

代码审查的实质内容可以概括为以下几点：

1. 纯粹针对语法吹毛求疵的评论会带来负面体验。风格和语义并非一回事。（有趣的是，5% 的受访者在负面语境中使用“吹毛求疵”一词来描述代码审查评论。）
2. 我们互相审查代码时使用的措辞确实很重要。不友善的审查可能会摧毁一个人的自信心。

我们怎样才能做得更好？

尽管这些数据可能不是最完整、最全面，甚至不是最准确地代表我们行业的代码审查文化，但有一件事似乎是合理的：我们都应该重新审视我们团队和更大社区内的代码审查流程。

这项匿名调查的回复凸显了审查流程对工程团队成员的巨大影响：

一次糟糕的代码审查差点让我离开公司。一次优秀的代码审查让我感觉更有信心去应对未来的项目。

确实，对正式的代码审查有一定的了解非常有益，而且在统计上也具有说服力；Steve McConnell 的研究和这项小型调查似乎都支持这一事实。但是，仅仅建立代码审查文化然后就不再考虑它是不够的。事实上，如果团队成员只是简单地进行代码审查，那么整个团队可能会受到损害，甚至会感到沮丧。

相反，对我们的集体代码审查文化进行内省、反思和重新评估的行为将使我们能够建立在我们可能拥有的任何类型的正式审查流程之上。

换句话说，我们应当问自己，我们的代码审查是否有效，以及它们是否对整个团队以及组成团队的个人产生了影响。

改进代码审查流程的简单方法

有几种方法可以立即让您的团队的代码审查过程变得更加轻松愉快。以下是一些可以帮助您入门的方法：

• 实施linters或代码分析器（如果可用）以消除对拉取请求的语法注释的需要。
• 对每个拉取请求使用Github 模板，并附上一份清单，以便代码作者和审阅者轻松知道要添加什么以及要检查什么。
• 添加屏幕截图和详细说明，帮助向可能不熟悉的队友提供背景信息
• 目标是提交小而简洁的提交，并封装拉取请求，使其规模不大，从而更容易、更快地进行审查。
• 为每个 PR 指定特定的审阅者——“如果可能的话，可以指定多个。确保审阅者的角色在各个级别的工程师之间平均分配。”

越难的事情——但也是最重要的

一旦你完成了一些唾手可得的成果，你还可以参与实现一些更大的改变。如果你想改变你的代码审查文化，这些实际上是最重要的事情。

并且，让我警告你：这可能就是让它们如此困难的原因。

• 培养团队的同理心。实现这一点的最大重担落在资深、经验丰富的工程师身上。要与团队或行业的新人建立同理心。

• 推动一种重视脆弱性的文化——无论是在行动上还是在言语上。这意味着要重新评估拉取请求评论中使用的语言，识别审查何时可能陷入恶性循环，并确定何时应进行线下对话，而不是公开质疑代码作者。

• 进行沟通。让你的团队坐下来，创建一个 Slack 频道，或者创建一个匿名调查问卷——无论哪种方式最符合你们团队的文化。进行一次对话，让每个人都能轻松地分享他们对当前代码审查流程的满意度，以及他们希望团队做出哪些改进。

我把最重要的一点留到了最后，因为说实话，如果你能坚持读到这里，你肯定非常渴望改变现状。这真的是一件非常好的事情™！不过，最终，与你的团队进行沟通才是实现这一改变最重要的第一步。

这份调查回复比我之前做的总结得更好：

理论上我喜欢代码审查。但实际上，代码审查的好坏取决于负责以正确方式进行审查的团队。

资源

如果您想查看更多精选的匿名调查回复，您可以查看该项目附带的网站：

更好的代码审查

致谢

首先，我非常感谢数百名开发人员花时间和精力回答我的调查。

非常感谢Kasra Rahjerdi，他帮助我分析了调查的回复并创建了该项目中的许多图表。

感谢Jeff Atwood撰写的关于同行评审的文章，感谢 Karl Wiegers 在《人性化同行评审》一书中所做的工作，以及 Steve McConnell 在《代码大全》中对代码审查流程进行的深入研究。希望您能考虑阅读他们的作品或购买他​​们的书籍来支持这些作者。

这篇文章最初发表在medium.com上