代码评审的主要目的是确保代码库的整体质量随时间推移逐步得到提升。这是一项艰巨的任务，因为代码库整体质量常常会随着每次提交代码质量的小幅下降而退化。代码评审就是确保每个CL（变更列表）的质量，保证代码库整体质量不会随着时间的推移而下降。

每个企业甚至每个团队都有自己的code review方式，无所谓好坏，都可以互相借鉴，取长补短。谷歌的code review经验实践一直是业界比较认可的方式，来看看谷歌怎么做的吧。

代码评审准则

一般来说，如果 CL 达到可以提升系统整体代码质量的程度，就可以让它们通过了，即使它们可能还不完美。这是所有代码评审准则的最高原则。

当然，也有例外的时候。例如，如果 CL中包含了系统不需要的功能，那么即使代码写得很好，评审人员也可以拒绝让它们通过。

因为这个世界上没有“完美”的代码，只有更好的代码。评审人员不应该要求开发人员对 CL 中的每一个微小部分都进行细致入微的打磨，而应该在满足需求和变更重要性之间做出权衡。评审人员不应该过度追求完美，而应追求持续改进。如果一个 CL 能够从整体上提高系统的可维护性、可读性和可理解性，那它就不应该仅仅因为它不够“完美”而被延迟几天甚至几周。

以上是代码评审的大指导方向。

代码评审原则

我们再来看看具体的细分原则：

1.客观的技术和数据比个人意见和偏好更重要

2.在代码风格方面，可以参考（谷歌风格指南）。任何没有在这个风格指南中出现的东西（比如空格等）都属于个人偏好。

3.如果没有其他适用的原则，评审人员可以要求开发人员与当前代码库保持一致，只要不破坏系统的整体代码质量。

代码评审究竟要关注哪些方面？

功能

这个 CL 是否有意义？对户来说有好处吗？

设计

代码评审中最重要的部分是 CL 的总体设计。

我们要考虑 CL 中不同代码段之间的交互是有意义的吗？

这个变更应该属于代码库，还是属于某个包？

它与系统的其他部分可以良好地集成吗？

现在是引入这个变更的好时机吗？

评审人员要警惕过度设计，鼓励开发人员只解决现在需要解决的问题，而不是将来可能需要解决的问题。

未来的问题应该在它们出现之后再去解决，因为到了那个时候我们可以看到它们的实际状况和需求。

过度设计是一种特殊的复杂性，开发人员把代码写得比实际需要的更通用，或者增加了系统当前不需要的功能。

复杂性

CL 比实际需要的更复杂吗？

从每一层面检查 CL，细到每一行代码，它们是不是太复杂了？“太复杂”通常意味着“阅读代码的人难以很快理解它们”，也意味着“开发人员在调用或修改这些代码时可能会引入 bug”。

测试

要求开发人员进行单元测试、集成测试或端到端测试。

一般来说，CL 中应该包含测试，除非这个 CL 只是为了处理紧急情况。

请记住，测试代码也是需要维护的。

命名

开发人员是否使用了良好的命名方式？

注释

开发人员有没有用自然语言写出清晰的注释？

他们所写的注释都是必需的吗？

代码风格

要确保 CL 遵循了适当的指南。

请不要只是基于个人偏好来阻止 CL 的提交。

开发人员不应该将风格变更与其他变更放在一起，这样很难看出 CL 发生了哪些变化，导致合并和回滚变得更加复杂。

如果开发人员想要重新格式化整个文件，让他们将重新格式化后的文件作为单独的 CL，并将功能变更作为另一个 CL。

文档

如果 CL 导致用户构建、测试、交互或发布代码的方式发生了变化，请确保相关的文档也得到了更新，包括 README、g3doc页和其他生成的参考文档。

如果 CL 有移除或弃用代码，请考虑一下是否也应该删除相关的文档。

查看每一行代码

查看每一行代码，不要只是粗略地扫一下类、函数或代码块，并假定它们都能正常运行，至少应该要理解这些代码都在做些什么。

如果代码很复杂或者你难以快速看懂它们，导致评审速度变慢，要让开发人员知道，并在进行进一步评审之前让他们做一些澄清。如果你看不懂这些代码，其他开发人员很可能也看不懂。因此，要求开发人员澄清代码其实也是在帮助未来的开发人员更好地理解代码。

如果你理解代码，但又觉得没有资格做代码评审，可以确保有资格的 CL 评审人员在代码评审时考虑到了安全性、并发性、可访问性、国际化等问题。

上下文

代码评审工具通常只显示被修改的代码，但有时候你需要查看整个文件，确保代码变更是有意义的。

例如，你可能只看到新添加了四行代码，但如果你看一下整个文件，会发现这四行代码位于一个 50 多行的方法中，这个时候需要将这个方法拆分为更小的方法。

好的一面

代码评审通常只关注错误的东西，但其实也应该鼓励和赞赏好的代码实践。

如果你在 CL中看到一些不错的东西，要让开发人员知道，特别是当他们以一种很好的方式解决了问题。

有时候，让开发人员知道他们做对了事情比让他们知道做错了事情更有价值。

因此总结下来，在进行代码评审时，你要确保：

良好的代码设计

功能对用户来说是有用的

UI 变更应该是合理的

并行编程是安全的

代码复杂性不要超过应有的程度

不需要实现可能会在未来出现的需求

有适当的单元测试

精心设计的测试用例

使用了清晰的命名方式

清晰而有用的代码注释

恰如其分的代码文档化

代码要遵循风格指南，而不是个人喜好

查看上下文，检查每一行代码

为表现不错的开发人员点赞

评审流程

知道了代码评审过程中要关注哪些点之后，是时候了解下代码评审流程了。

第一步：从整体查看代码变更

先看一下 CL 描述，看看这个 CL 做了些什么。做出这个变更有意义吗？如果这个变更是不必要的，请立即做出回复，并解释为什么不应该发生这个变更。在你拒绝这样的变更时，可以向开发人员建议他们应该做些什么。

第二步：检查 CL 的主要部分

找到 CL 的主要文件。通常一个 CL 会有一个包含了主要逻辑变更的文件，也就是 CL 的主要部分。先看看这些主要部分，有助于了解整个上下文，加快代码评审速度。如果 CL 太大，以致于你无法确定哪些部分是主要的，可以询问开发人员，或者让他们把 CL 拆分成多个 CL。

如果 CL 的主要部分存在严重的设计问题，要立即回复开发人员，即使你还没有时间检查 CL 的其余部分。这个时候检查 CL 的其余部分可能是在浪费时间，因为如果主要部分存在严重的设计问题，那么其他部分就变得无关紧要了。

第三步：按照适当的顺序检查 CL 的其余部分

在确认整体 CL 没有严重的设计问题之后，试着按照某种逻辑顺序来检查其他文件，确保不会错过任何一个需要检查的文件。通常，在你检查完主要文件之后，按照代码评审工具显示它们的顺序来浏览每个文件就可以了。你也可以在检查主要代码之前先查看测试代码，这样可以对代码变更有一个大致的概念。

代码评审回推

有时候，开发人员会回推代码评审。他们可能不同意你的意见，或者他们抱怨你太严格了。

谁是对的？

如果开发人员不同意你的意见，先花点时间想一下他们是不是对的。通常，他们比你更熟悉代码，所以可能对代码的某些方面更了解。他们的论点有道理吗？从代码质量角度来看，他们的回推是有道理的吗？如果是，就让他们知道他们是对的，这个问题就解决了。

然而，开发人员并不总是正确的。在这种情况下，评审人员要进一步解释为什么他们的建议是正确的。

如果评审人员认为他们的建议可以改善代码质量，并认为评审所带来的代码质量改进值得开发人员做出额外的工作，那么他们就应该坚持。改善代码质量往往是由一系列的小步骤组成的。

有时候你需要花很多时间反复解释，但要始终保持礼貌，并让开发人员知道你知道他们在说什么。

激动的开发人员

有时候，评审人员会认为如果他们坚持要开发人员做出改动，会让开发人员感到不安。开发人员有时候确实会感到沮丧，但这种感觉通常都很短暂，之后他们会非常感谢你帮助他们提高了代码质量。如果你在评审过程中表现得很有礼貌，开发人员一点都不会感到不安，这种担心可能是多余的。通常，令开发人员感到不安的是你写注解的方式，而不是你对代码质量的坚持。

稍后再解决

一种常见的回推原因是开发人员希望尽快完成任务。他们不想经过一轮又一轮的代码评审，他们说他们会在后续的 CL 中解决遗留问题，你现在让 CL 通过就可以了。一些开发人员会做得很好，他们在提交 CL 后立即就开发后续的 CL。但经验表明，开发人员开发原始 CL 的时间越长，他们进行后续修复的可能性就越小。除非开发人员在提交 CL 之后立即进行修复，否则在通过之后通常不会再去做这件事情。这并不是因为开发人员不负责任，而是因为他们有很多工作要做，而修复工作通常会被遗忘。所以，最好让开发人员马上把 CL 修复掉。

如果 CL 引入了新的复杂性，在提交之前必须将其清理掉，除非是紧急情况。如果 CL 暴露了一些目前还无法解决的问题，开发人员需要把 bug 记录下来，并将其分配给自己，这样它就不会被遗漏。他们还可以在代码中加入 TODO 注释，指向已经记录好的 bug。

抱怨评审太严格

如果你之前的代码评审很放松，然后突然变得严格起来，可能会引起一些开发人员的抱怨。不过没关系，加快代码评审速度通常会让这些抱怨逐渐消失。

有时候，这些抱怨可能需要几个月的时间才能消除，但开发人员到最后通常会看到代码评审的价值，因为他们看到了严格的代码评审有助于产出优秀的代码。有时候，抗议最大声的人甚至会成为你最坚定的支持者。

解决冲突

如果你遵循了上述方法，但仍然会在评审过程中遇到无法解决的冲突，请再次参阅代码评审标准，了解那些有助于解决冲突的指导原则。

代码评审的速度

为什么代码评审要快速进行？

在谷歌，对开发团队的整体交付速度（而不是针对个体开发人员写代码的速度）进行了优化。个体开发速度也很重要，但其重要性比不上整个团队的开发速度。

如果代码评审的速度很慢，就会发生以下这些事情：

团队的整体开发速度降低了。如果个体开发人员无法快速地对评审做出响应，可能是因为他们有其他事情要做。但是，如果每个 CL 都要等待一次又一次的评审，那么其他成员的新特性和 bug 修复就会被延迟，可能是几天、几周甚至是几个月。

开发人员开始对代码评审流程提出抗议。如果评审人员要隔几天才回复一次，但每次都要求对CL 进行重大修改，开发人员可能会觉得很沮丧。通常，他们会抱怨评审人员太过严苛。如果评审人员能够快速提供反馈，抱怨就会消失，即使他们要求做出的修改是一样的。代码评审过程的大多数抱怨实际上可以通过加快评审速度来解决。

代码质量受影响。如果评审速度很慢，开发人员的压力也会随之增加，因为他们不能提交不甚完美的 CL。缓慢的评审流程还会阻碍代码清理、重构和对现有 CL 做出进一步改进。