如何进行出色的代码评审

这篇文章最初发表在我的博客上。

为了进行出色的代码审查，您需要知道为什么要进行代码审查。

我们为什么要审查代码？

我们都知道代码审查很重要，但为什么呢？它们可以发现一些 bug，并防止糟糕的设计决策进入生产环境，但这就是它们唯一的作用吗？

根据我的经验，出色的代码审查有三个重要目标：

1. 让团队及时了解不断变化的代码库。
2. 提高工程质量。
3. 为开发人员提供反馈。

跟上不断变化的代码库

代码审查迫使开发人员阅读他们自己未参与的代码部分。这提升了团队对代码库的整体了解。因此，任务规划更加顺畅，时间/复杂性估算的准确性也得到了提升。

一、先找出原因

为了能够理解代码更改，您必须首先了解更改的原因。

我们大多数人都会使用某种问题跟踪系统。找到与此代码变更相关的问题/工单，并仔细阅读，包括注释。如果有拉取请求的描述，也请阅读。这不仅能为你提供必要的背景信息，帮助你理解变更本身，还能判断变更是否符合要求。

例如，如果没有上下文，将点硬编码为数字的小数分隔符对于仅限美国的应用程序来说似乎是合理的，但如果您阅读了票据，您就会知道您的公司正在扩展到其他国家，现在用户的语言环境需要参与选择小数分隔符。

如果没有问题/工单，也没有 PR 描述，请写下您自己的描述并将其作为评审的一部分。如果您误解了目的，被评审人很可能会指出这一点。

二、阅读，不要浏览

不要满足于简单的“看起来不错”。所有格式良好的代码，当你浏览时看起来都很棒！你需要阅读并真正理解这些变化。是的，这很困难。是的，这会花费更多时间。是的，这是值得的。这还能增加你发现那些不太明显的项目质量问题的机会。

提高项目质量

在投入生产之前让另一个人检查代码是发现一些错误并确保代码可维护的好方法。

III. 始终运行代码

只需修改一行代码？运行代码。只需修改 CSS？运行代码。只需翻译？运行代码。

别自欺欺人地以为每个人每次修改代码后都会编译运行项目。我们应该这么做……但我们却没有。你知道，有些时候你离下班只剩5分钟了，你只需要再做一点小改动，却没有时间去验证。

确保项目仍然能够编译并运行，且不抛出异常，这是你作为审阅者至少应该做的。这只是一个简单的健全性检查。所以你应该这么做。

如果引入了任何用户可见的更改，请务必亲眼查看。点击查看新的 UI 组件。

运行测试。是的，我知道 CI 应该这样做。但也许有些不稳定的测试会偶然在 CI 上通过？或者，也许有一个 bug 只出现在你的 macOS 上，但 CI 运行的是 Linux？也许有人没有遵循正确的测试文件命名模式，导致测试运行器完全忽略了该文件？

这些变更是否包括数据库迁移？通过回滚、签出master并检查项目是否仍在运行来结束审核流程。

四、不要吹毛求疵

有人在应该只插入一行的地方插入了两行空行？别提了。被审阅者修复你代码中问题的耐心是有限的，你肯定不想把耐心浪费在那些琐碎的问题上。

相反，尝试找到一个自动化解决方案，确保自动检测所有这些琐碎的细节，并建议您的团队开始使用它。

五、关注无法自动检测的可读性问题

一旦您的自动化工具能够验证函数名称的长度和正确的缩进，您就可以专注于只有人类才能捕捉到的重要内容。

这些函数/变量名对你来说有意义吗？它们是否使用了项目常用的词汇？它们会让人困惑吗？是否存在一些容易产生歧义的缩写？

有趣的故事：我曾经花了 10 分钟的时间相信我正在开发的应用程序具有一项功能，即访客用户（付费订阅用户邀请的非付费用户）可以邀请更多访客用户，因为有人将 命名为变量sub_guest而不是subscription_guest。

为开发人员提供反馈

好的反馈能够强化某人的好习惯，并帮助他们发现并改掉坏习惯。为了让你的反馈产生任何影响，你需要确保你的反馈被认真倾听。

六、总是说些好话

我不在乎代码改动有多琐碎。你总是可以说些积极的话。积极的反馈越具体越好。

如果您以前从未这样做过，可能很难想出办法，因此这里有一些通用的例子来激发您的灵感：

干得好！我尽力在你的代码里找bug了，但没找到。

感谢您修正了那个拼写错误，我不知道这是该词的正确拼写。

我喜欢你给这个函数命名的方式。它的作用一目了然。

哇，太棒了。我之前都不知道我们的编程语言/框架/库有这个功能。

我喜欢你把所有新代码都添加到一个新文件中。我本来想把它放在现有文件中，但那样会让现有文件变得太大。

如果你害怕因为欣赏一些琐碎的事情而显得愚蠢，那就问问自己，当你收到代码评审时，你的感受是什么。你兴奋吗？还是你害怕它，因为你知道在最好的情况下，它只是中立的默许，但在最坏的情况下，它会充满严厉的批评？你真的希望你的同事仅仅因为你害怕显得愚蠢而害怕接受你的评审吗？

积极反馈的另一个重要作用是，它使被评价者更愿意接受你的负面反馈。

你家里有没有哪位朋友，每次见到你都会抓紧时间抱怨你？你又胖了，你兄弟姐妹的收入比你高，你应该多笑一笑……你又怎么可能把他们的抱怨当成激励你进步的动力呢？一点儿也不可能？其实，几个月都收到同一位同事的负面反馈，感觉也差不多。

七、避免使用评判性和颐指气使的语言

避免听起来评头论足和颐指气使的最简单方法是假设你错了，而被评审者是对的。然后提出一些问题，帮助你认识到自己错在哪里。毕竟，他们花在这段代码上的时间可能比你花在评审上的时间要多得多。他们更有可能理解细节。

不要说“ 不要这样做，要那样做“你不想培养同事服从命令的能力。你希望他们具有创造性和批判性思维。此外，没有人喜欢被人指使。

应该说“你觉得……怎么样？”。表明你愿意交谈。如果人们参与了决策，他们会更有动力去做某事。

不要说“ 你为什么不...这句话暗示你的解决方案是显而易见的选择，而被评审者不采用它是愚蠢的。这可能会引发一种防御性反应，让原本聪明通情达理的人热情地捍卫一个有缺陷的立场。

相反，应该说“你考虑过……吗？”，解释为什么你的解决方案在这种情况下是有利的，然后友好地邀请对方证明你错了，例如“……或者我遗漏了什么？”。

现在，我并不是说你总是错的。但你肯定也并非总是对的。这种方法能让你和被评估者更容易承认彼此的错误，而不会丢脸。

GitHub PR 评论

GitHub 有一项功能，允许您将拉取请求审核标记为评论、接受或请求更改。

我倾向于避免提出修改请求，除非我绝对确定需要修改。提出修改请求是种强势的言辞，没有多少辩论的余地。我会先进行评论审核，直到所有问题都得到解决，然后才会接受。

概括

出色的代码审查需要花费时间，但它们不仅会改善您的项目，还会改善您对项目的了解以及您与团队的关系。

记住：

• 跟上不断变化的代码库
  • 一、先找出原因
  • 二、阅读，不要浏览
• 提高项目质量
  • III. 始终运行代码
  • 四、不要吹毛求疵
  • 五、关注无法自动检测的可读性问题
• 为开发人员提供反馈
  • 六、总是说些好话
  • 七、避免使用评判性和颐指气使的语言