6 个实用的 Code Review 实践技巧

本文最初发布于 Shopify 博客，经原作者授权由 InfoQ 中文站翻译并分享。

Code reviews 是打造高效团队的重要方面，这已经成为共识。关于这个主题，有许多文章曾经讨论过，比如这篇论文——《An Empirical Study of the Impact of Modern Code Review Practices on Software Quality》。现实中，许多企业的无数团队都进行过某种形式的 code reviews。

而实际情况是，code reviews 刚开始时，人们的激情高涨，之后，code reviews 则流于形式，或者要么反馈不清晰、要么让人难以执行。长久以往，这让团队错失了加快学习、分享知识的机会，最终难以提高代码的质量。

在 Shopify，我们不仅立足长远，而且希望追求发展更快。以我们的经验来看，优秀的 code reviews 实践对工程师的成长和我们所打造的产品质量有着巨大影响。

噩梦般的编码经历

这样一个场景相信很多人都很熟悉：

你刚刚加入一个新团队，领导很快给你分配了一个编码任务。作为新人，你特别想表现自己，因为你想秀一下自己的编码水平。于是，你接下来做了这些事：

1.你为了完成任务疯狂地敲了三周代码；

2.你将一个包含大约 1000 行新代码的 Pull Request 提交评审；

3.你收到两条关于 code style 的评论，以及一个关于评审人表示他看不懂这些代码用途的问题；

4.你修复 code style 并回答评审人的问题，然后评审人通过你写的代码；

5.你把代码分支合并到 Master，双眼紧闭，紧握着拳头，紧咬牙关等待着结果。几分钟后，CI 完成。幸好，Master 没有崩溃。然而…

6.此后 6 个月，你一直战战兢兢，不知道代码何时会崩溃，以及以什么方式崩溃。

你可能经历过上述噩梦般的经历，那我们谈谈怎样改进这个流程吧！

实用的 Code Review 实践

在 Shopify，我们看重交付速度、学习以及长期发展。这些价值观虽然有时会产生冲突，但却引导我们不断尝试许多新技术，并推动团队变革。

我在本文总结了一系列 Shopify 内部使用的实用技巧。借助这些技巧，我们能交付经得起时间考验的有价值的代码。

术语说明：我们将 Pull Requests（PR）定义为合并到基础分支前进行 code reviews 的一个工作单元。Github 和 Bitbucket 的用户对这个术语很熟悉。

1. 将 Pull Request 拆分为较小的代码段

这个方法很简单，可以成为提高 code reviews 工作流程最有用的技术。它之所以有效，主要有两个原因：

• 评审人心理上更容易接受开始和完成一小块代码的评审工作。更大的 PR 自然会让评审人推迟和拖延评审，并且在评审过程中被打断的可能性更大。

• 作为一名评审人，如果 PR 太长，就很难深入进去。要检查的代码越多，我们越需要耗费更多脑力来理解整个代码块。

将 PR 拆分为更小的代码段，让你有更多机会在更短时间内得到更深入的评审。

目前，我们无法设置一个适用于所有编程语言和所有类型工作的通用标准。对于内部的数据工程项目，我们原则上是要将 PR 控制在 200-300 行代码。如果超过这个阈值，我们一般会将它拆分成更小的块。

当然，我们也要注意不要将 PR 拆分得过小，因为这意味着评审人可能需要检查好几个 PR 才能理解整体逻辑。

2. 使用 Draft PRs

你听过造一辆汽车与画一辆汽车的比喻吗？这个比喻是这么说的：

1.    用户要你造一辆车；

2.    6 个月后，你造了一辆漂亮的保时捷；

3.    你向用户展示这辆车后，他们问你这辆车能不能放得下他们的 5 个孩子和冲浪板。

显然，这里的问题在于目标不清晰，团队没有收集到足够的反馈就直接构建解决方案。如果在第一步后，我们先画一幅汽车的草图，并将其展示给用户，他们会问相同的问题，这样就可以进一步了解客户需求。如此，就为我们节省了 6 个月的工作量。软件也不例外，我们可能会犯同样的错误，在用户不需要的特性或模块上投入大量工作。

在 Shopify，一般用 Work In Progress (WIP) PRs 来获得早期反馈，其目标是验证方向（算法、设计、API 等等选择）。尽早变更可以避免在细节、修饰、文档等方面浪费精力。

作为一名写代码的人，这意味着你要对变更工作方向持开放态度。

在 Shopify，我们信奉的原则是允许大家有自己的理解，但不固执己见。我们希望大家能在有充足理由的情况下自信地做出决定，但同时也能乐于学习其他更好的新方案。在实际工作中，我们使用 Github 的 Draft PRs，它们明确表明这项工作仍在流程中流转，Github 不允许你合并一个 Draft PR。其他工具可能有类似的功能，至少你创建正常 PR 时可以加上一个 WIP 标签，以明确表示该工作还处于前期阶段。这将帮助你的评审人专注于适当的领域，提出适当的反馈。

3. One PR Per Concern

除了行数外，需要考虑的另一个维度是你的工作单元试图解决的问题数量。一个关注点可以是一个特性、一个错误修复、一个依赖项升级、一个 API 变更等等。你是否在重构的同时引入一个新特性？一次修复了两个错误？同时引入了类库升级和新的服务？

把 PR 分解为一个个单独的关注点，它会产生下列影响:

• 更独立的评审单元，这意味着更好的审查质量；

• 受影响的人更少，因此可以聚集在更少的几个专业领域中；

• 原子性回滚，可以回滚小的 commit 或 PR。这是很有价值的，因为如果出了问题，就更容易确定错误是在哪里引入的，以及回滚哪些部分。

• 将易事和难事分开。假设有一个新特性，需要重构一个频繁使用的 API。你可以更改这个 API，升级十几个调用的站点，然后实现这个特性。你的变更中有 80%不是功能上的变更，明显可以忽略掉，而 20%是需要仔细注意测试覆盖率、预期行为、错误处理等等的新代码，并且可能要经过多次修订。对于每一个修订，评审人都需要浏览所有的修改以找到相关的部分。通过将其分成两个 PR，很容易就可以快速完成大部分工作，并优化评审工作，将主要精力投入到难点上。

如果你最终拿到手的 PR 包含多个关注点，那么你可以将其分解为多个单独的块。这样能针对每一块进行单独的评审，每次评审的迭代周期可以更快，从而加速这个 PR 的总体评审周期。通常情况下，有一部分工作能先快速完成，避免代码烂到不能用以及引起合并冲突。

将 PR 分解成单独的关注点

上例的 PR 包含三个不同的关注点，我们将其进行拆分。可以看到，每个评审人需要检查的上下文少了许多。最重要的是，只要其中任何一个部分的评审完成，代码作者就能一边等待其他评审反馈，一边着手处理已经反馈的问题。在最极端的情况下，代码作者会陆续收到各个部分的评审反馈，几乎可以不间断地处理完这一系列 PR，而不是完成初稿后，等上几天（已经去忙其他的事），然后最后再返回头来处理反馈意见。

4. 专注代码，而不是人

专注于代码，而不是人，这条实践谈的是人与人之间的沟通方式和关系。从根本上讲，这是提倡我们尝试把注意力集中在如何改进产品上，避免作者将评审意见当作对他个人的批评。

以下是一些你可以遵循的技巧：

• 评审人可以这样想：“这是我们自己的代码，我们该如何改进它呢？”

• 提出肯定意见！如果你看到有些代码部分写得不错，就加条评论表扬一下。这能让代码作者继续保持好的一面，并有助于他在心理上更容易接受改进建议。

• 代码作者不妨这么想，评审人的出发点肯定是好的，不要将评论当作是对个人的批评。

• 下表列出了一些存在不足的评审反馈，以及如何按以上建议进行重写的建议。

归根结底，code review 给我们提供了互教互学的机会，我们应该对此持开放欢迎的态度。

5. 挑选合适的评审人

决定由谁来评审你的工作通常很有挑战性。以下问题可作为参考：

• 谁具备你正在构建的特性或组件的上下文？

• 谁精通你正在使用的语言、框架或工具？

• 谁对这一主题知之甚深，有自己的理解？

• 谁关心你所写代码的结果？

• 谁应该学习这些东西？或者，如果你是一名正在评审“老鸟的菜鸟程序员”，不妨抓住这个机会多多提问学习。别怕你的问题太幼稚，一个强大的团队会找时间来分享知识。

无论你的团队遵循哪些原则，请记住，作为一名代码的作者，你有责任寻求并接受适当的人对你的代码进行高质量的 code review。

6. 给评审人提供关键的上下文

最后但同样非常重要的一点是，你的 PR 描述至关重要。这取决于你选择的评审人，不同的人会有不同的上下文。代码的作者有责任提供关键信息或更多上下文的链接，帮助评审人能够反馈有价值的意见。

你可以把以下问题放到你的PR模板中：

• 为什么这个 PR 是必要的？

• 谁会从中受益？

• 可能会出什么问题？

• 你还考虑过其他方法吗？你为什么决定采用这种方法？

• 这对其他系统有什么影响?

好的代码不仅没有错误，还非常有用。作为一名代码的作者，请确保你的 PR 描述将代码与团队目标联系起来，最好能与待办事项中的特性或缺陷描述联系起来。作为评审人，会先评审 PR 描述，如果它不够完整，你是无法针对未定义的目标来判断代码是否适当的，不如在评审代码前就把它打回去。请记住，有时代码审查的最佳结果是认识到根本不需要这些代码！

我们会有哪些收获?

通过采用上面的一些技术，你可以在很大程度上影响软件构建过程的速度和质量。但除此之外，还有潜在的文化影响：

• 团队将达成共识。团队会更了解你的工作，除你之外，其他团队成员可以完善代码库的这一部分。

• 团队将共同承担责任。如果出现问题，不只是某个人的代码需要修复，而是整个团队的代码都需要修复。

任何团队成员都应该能够休上几天假，他几天不工作不会让公司面临风险，也不会因为担心世界末日而不停地去看电子邮件。

个人可以如何改进团队的代码审查流程？

如果你是团队主管，不妨开始尝试这些技巧，找出适合你所带团队的方法。

如果你是独立贡献者，可以与主管讨论一下为什么你认为代码审查技术很重要，以及它如何提高效率和帮助团队。

在下次一对一交流或团队会议上，探讨一下这个问题。

英文原文：

Great Code Reviews—The Superpower Your Team Needs