本文翻译自：Google 工程实践文档

许可：CC-By 3.0 License

前言

在 Google 有许多通用的工程实践，几乎涵盖了所有语言和项目。此文档为长期积累的最佳实践，是集体经验的结晶。我们尽可能地将其公之于众，你的组织和开源项目也会从中受益。

Google 的 Code Review 指南，实际包含以下两个单独的文档：

术语

其中一些文档中使用了一些 Google 内部术语，我们在这里为外部读者澄清：

  • CL: “Change List” 的缩写，表示一个已提交到版本控制或正在进行代码审查的独立更改。在其他组织内可能会将其称为 “change”、“patch” 或 “pull request”。
  • LGTM: “Looks Good to Me” 的缩写，审查者说了这句代码变更通过了。

介绍

Code Review 是除了代码作者之外，其他人检查代码的过程。

Google 通过 Code Review 来维护代码和产品质量，此文档则是 Google Code Review 流程和政策的规范说明。

代码审查者应该关注哪些方面？

Code Review 时应该关注以下方面：

  • 设计：代码是否经过精心设计并适合你的系统？
  • 功能：代码的行为是否与作者的意图相同？代码是否可以正常响应用户的行为？
  • 复杂性：代码能更简单吗？将来其他开者能轻松理解并使用此代码吗？
  • 测试：代码是否具有正确且设计良好的自动化测试？
  • 命名：开发者是否为变量、类、方法等选择了清晰的名称？
  • 注释：注释是否清晰有用？
  • 风格：代码风格是否遵守了 Google 代码风格指南？
  • 文档：开发者是否同时更新了相关文档？

选择最合适的审查者

一般而言，你希望找到能在合理的时间内回复你的代码提交的最合适的审查者。

最合适的审查者应该是能彻底了解和审查你代码的人。他们通常是代码的所有者，可能是 owner(s) 文件中的人，也可能不是。有时 CL 的不同部分可能需要不同的人审查。

如果你找到了理想的审查者但他们又没空，那你也至少也要抄送他们。

面对面审查（和结对编程）

如果你与有资格做 Code Review 的人一起结对编程了一段代码，那么该代码可被视为已审查。

你还可以进行面对面的代码审查，审查者提问，CL 的开发者作答。

代码审查者指南

Code Review 的标准

Code Review 的主要目的是确保逐步改善 Google 代码库的整体健康状况。Code Review 的所有工具和流程都是为此而设计的。

为了实现此目标，必须做出一系列权衡。

首先，开发者必须能够对代码进行优化。 如果你从未向代码库提交优化变更，那代码库永远不会改进。此外，如果审查者对代码吹毛求疵，那么开发者可能未来会不再提交优化变更。

另外，审查者有责任确保随着时间的推移，CL 的质量不会使代码库的整体健康状况下降。 这可能很棘手，因为通常情况下，代码库的健康状况会随着时间的推移而下降，特别是在团队有严格的时间要求时，他们必须走捷径才能完成目标时。

此外，审查者应对正在审核的代码负责并拥有所有权。 审查者希望确保代码库保持一致、可维护及 Code Review 的关注点中提到的所有其他内容。

因此，我们将以下规则作为 Code Review 中期望的标准：

总体来说，只要 CL 真的可以提高系统的整体代码健康状态，即使这个 CL 并不完美，审查者也应该倾向于批准该 CL。

这是所有 Code Review 指南中的核心原则。

当然，也有一些限制。例如，如果 CL 添加了审查者认为系统中不需要的功能，那么即使代码设计良好，审查者依然可以拒绝批准它。

这里有个关键点，没有『完美的』代码 – 只有『更好的』代码。审查者不该要求开发者在提交变更前仔细清理、润色 CL 的每个角落。相反，审查者应该权衡改进需求和进度推进之间的平衡。与其追求完美，不如聚焦持续优化。只要这个 CL 能提升代码整体的可维护性、可读性和可理解性，就不应该因为不够『完美』而延误数日甚至数周才合并。

注意： 本文内容绝不应被用作提交会使系统整体代码健康状况下降的 CL 的理由。只有在 紧急情况 下才能这么做。

指导

Code Review 承担着重要的教学功能——开发者能借此了解语言特性、框架机制或通用软件设计原则的新知识。留下评论可以帮助开发者学习新东西，知识共享本身就是持续提升系统代码健康度的重要环节。 但需注意，如果你的评论纯粹是教育性的，且对于本文档中描述的标准并不重要，请在其前面添加 “Nit:” （Nitpick 吹毛求疵的缩写）或以其他方式表明可以不必在此 CL 中解决它。

原则

  • 技术事实与数据高于观点和个人偏好。

  • 在代码风格问题上，Google 代码风格指南 具有绝对权威。凡未纳入风格指南的纯样式问题（如空格使用等）均属个人偏好范畴，但需保持与既有代码风格一致。若无先例可循，则应采纳开发者的风格方案。

  • 软件设计层面的问题几乎从不属于纯样式或个人偏好范畴，其判断应基于底层设计原则进行权衡，而非个人喜好意见。 若存在多个有效方案，且开发者能通过数据或扎实的工程原理证明多种方法均属合理，审查者应尊重开发者的选择。否者，遵循软件设计的标准原则。

  • 如果没有其他规则适用，审查者可要求开发者与现有代码库保持一致，只要这个要求不会使代码整体恶化。

解决分歧

如果在 Code Review 过程中有任何分歧，第一步应该始终是开发者和审查者基于本文中的 代码变更者指南 和 代码审查者指南 达成共识。

当共识难以达成时，进行面对面会议或视频会议往往比仅通过代码评审注释解决分歧更有效。（若采用此方式，务必将讨论结果记录到提交评论中，以供后续查阅。）

若还是无法解决分歧，最常见的处理方式是升级决策。通常可采取以下路径：发起更广泛的团队讨论、征求技术主管意见、让代码维护者判断或找工程经理帮助。不要因为开发者与审查者没达成一致就让 CL 在那搁置。

Code Review 的关注点

注意：当考虑以下每一点时，一定要先确保考虑到上面 Code Review 的标准。

设计

Review 中最重要的事情是 CL 的整体设计。CL 中各种代码片段的交互是否有意义？该变更是属于你负责的代码库还是公共的 library 库？是否与你系统很好地集成？现在是添加这个功能的好时机吗？

功能

这个 CL 提交是否符合了开发者的设计意图？该设计意图是否对代码用户有益？这里『用户』通常既是最终用户（当他们受到更改的影响时）也包括开发者（他们将来必须『使用』此代码）。

原则上，我们要求开发者在提交 CL 前对代码进行充分测试以确保功能正确。但作为评审者，你仍应考虑边界情况、排查并发问题、尝试站在用户角度思考，并通过代码评审确保没有明显的 BUGs。

如果你正在审查的这个 CL 会对用户产生重大影响（如 UI 变更），那么你就很有必要验证 CL 的效果。仅通过代码阅读往往难以完全理解变更对用户的影响。如果在 CL 中打 patch 或自行部署验证不太方便，可以让开发者为你提供功能演示。

在 Code Review 中，如果 CL 中存在并行编程的提交，功能可靠性就特别重要。此类代码可能引发死锁或竞态条件问题。这些隐患很难通过简单运行暴露，通常需要有人（开发者和审查者）仔细推演确保没有引入问题。（这也提示我们应尽量避免采用可能产生死锁或竞态条件的并发模型——否则会使代码评审或理解代码变得非常复杂。）

复杂性

CL 是否比应有的更复杂？可以从以下几个层面进行审查：单行代码是否过于复杂？函数功能是否过于复杂？类结构是否过于复杂？所谓『过于复杂』，通常指 『代码阅读者无法快速理解』，也可能意味着 『开发者在调用或修改该代码时容易引入错误』。

其中一种复杂性就是 过度工程化（over-engineering），即开发者使代码过度通用，或添加了系统当前并不需要的功能。代码审查者应特别警惕过度工程。应倡导开发者专注于解决已知的问题，而不是推测未来可能需要解决的问题。待未来问题真正出现且其具体的需求形态明朗时，才是着手解决的最佳时机。

测试

可以根据代码变更性质提交对应的单元、集成或端到端测试，除 紧急情况 外，测试代码通常应该跟生产代码在同一个 CL 中提交。

确保 CL 中的测试代码准确、合理且有效。测试代码不会自我验证，我们极少为测试再编写测试，必须由人工审查保证测试的有效性。

当代码损坏时，测试真的会失败吗？如果代码发生变化时，测试是否会误报？每个测试是否都进行了简洁有效的断言？不同测试方法的测试是否适当隔离？

谨记：测试代码同样需要维护。不要因为测试不属于主二进制文件就降低对它的要求。

命名

开发者是否为所有内容都提供了清晰的命名？一个好的名字不能太短，它要足以阐明项目的内容或作用，但又不会太冗长，以至于难以阅读。

注释

请确认开发者是否采用了清晰易懂的中文或英文（语言跟代码仓库一致）注释？所有注释是否有必要？通常来说，注释的价值在于解释这段代码为什么存在，而不是复述这段代码的行为逻辑。如果代码本身无法清楚到解释自己时，那应先简化代码而不是依赖注释。当然存在些例外（比如正则和复杂算法经常需要通过注释来说明其运作机制），多数情况下，注释应该包含代码本身没法包含的信息，比如这段代码背后的决策推理。

同时应注意查看本次 CL 前已有的注释，或许存在可立即移除的 TODO 注释，或可能包含反对当前修改的建议性注释等。

注意：代码注释与类、模块或函数的文档不同，后者应该明确说明一段代码的目的，如何使用它，以及使用时的行为方式。

风格

Google 提供了所有主要语言的 Google 代码风格指南，甚至包括大多数小众语言。确保 CL 遵循适当的风格指南。

如果你想改进风格指南中没有的一些风格点，请在评论前加上 “Nit:"，让开发者知道这是一个你认为会改进代码的小瑕疵，但不是强制性。不要仅根据个人风格偏好阻止提交 CL。

CL 的作者不应该将重大的风格样式变更和其他变更结合在一块提交。它会让 CL 中难以看到正在变更的内容，会使合并和回滚更复杂，并导致其他问题。例如，如果作者想修改整个文件的代码风格样式，请让它仅将代码风格样式的修改作为一个 CL 提交，然后提交另一个关于功能变更的 CL。

一致性

如果现有代码与风格指南不一致怎么办？请根据我们的 Code Review 的标准，风格指南是绝对权威：如果风格指南要求某些内容，CL 应该遵守指南。

在某些情况下，风格指南仅提供建议性规范而非强制性要求。此时需要判断新代码应当遵循指南建议还是与周边代码保持一致。除非会导致显著的理解障碍，否则应优先遵循风格指南。

如果没有其他规则适用，开发者应当保持与现有代码的一致性。

无论哪种方式，都应鼓励开发者提交错误并添加 TODO 注释，以便后续清理存量代码。

文档

如果 CL 中更改了用户构建、测试、交互或发布代码的方式，请检查是否还更新了相关文档。包括 README、g3doc 文档和所有自动生成的参考文档。如果 CL 中删除或弃用了某些代码，需评估是否也应同步删除相应文档。若发现文档缺失，应及时要求补充。

每一行

通常情况下，请仔细审查分配给你的每行代码。有些东西，比如数据文件、生成的代码或大型数据结构，你可以快速扫过，但对于人工编写的类、函数或代码块，不要流于表面地扫视便假定内容正确无误。当然，不同代码需要的审查深度可能有所不同，这需要你自己判断，但至少应确保你理解了所有代码的实际功能。

如果代码难以理解导致评审太慢，那么你应该及时告诉开发者，并等他们澄清后再继续评审。在 Google，我们聘用的都是卓越的软件工程师，而你就是其中之一。如果你都读不懂某段代码，那么其他开发者很可能同样难以理解。因此，要求开发者澄清代码不仅有助于当前评审，更是在帮助未来可能接触这段代码的开发者。

如果你理解了代码，但你觉得没有资格进行某些部分的审查，请确保 CL 上有一个合格的审查者 ，特别是对于隐私、安全、并发、可访问性、国际化等复杂问题。

异常

如果不需要全面的评审所有代码应该怎么处理？比如你作为多名评审者之一，可能会被要求：

  • 仅评审大型变更中特定的文件。
  • 仅关注代码的特定方面（如高层设计、隐私或安全影响等）。

这种情况下，可以通过评论明确说明您已评审的范围。建议在附加说明的前提下给予 LGTM 评论。

如果你希望在确认其他评审者已完成对应部分审查后再授予 LGTM，应在评论中明确说明以设定预期。一旦 CL 达到理想状态，应该快速响应。

上下文

从更宏观的视角审查 CL 往往能发现关键问题。代码评审工具通常只会显示变更区域部分的几行代码。但有时你必须查看整个文件才能确保修改的合理性。比如，你可能只看到新增了四行代码，但当查看完整文件时，会发现这四行正位于一个长达 50行 的方法中，而该方法现在确实需要拆分成多个小方法。

在整个系统的背景下考虑 CL 也很有用。这个 CL 是改善了系统的代码健康状况，还是会使整个系统变得更加复杂、测试更少等？ 绝不能接受导致代码健康度恶化的 CL。 大多数系统的复杂性都是通过无数小变更累积形成的，因此防止新变更引入即便很小的复杂性也非常重要。

优秀实践

如果你在 CL 中看到一些优秀实践，请告诉开发者，尤其是当他们以很好的方式处理你的评论时。Code Review 通常只关注错误，但也应该鼓励和赞赏良好的实践。在指导方面，有些时候告诉开发者他们做对了什么比告诉他们做错了什么更有价值。

总结

在进行 Code Review 时，你应该确保：

  • 代码架构设计良好。
  • 该功能对代码用户是有好处的。
  • 任何 UI 变更都合理且视觉协调。
  • 任何并行编程都是安全的。
  • 代码复杂度控制在必要的范围内。
  • 开发者没有过度开发当前不需要的潜在功能。
  • 代码有适当的单元测试。
  • 测试设计完备。
  • 命名规范清晰明确。
  • 注释清晰有用，且大多数用来解释为什么而不是做什么。
  • 代码文档完善（通常采用 g3doc 形式）。
  • 遵循代码风格指南。

务必审查分配到的每行代码，结合上下文进行 Review ，确保代码健康度得到提升，并对开发者的优秀实践给予积极肯定。

Code Review 的步骤

知道了 Code Review 的标准和关注点，那么如何最有效管理分布在多个文件中的 CL 呢？

  1. 本次变更是否合理？描述 是否清晰？
  2. 首先关注核心的修改部分，审视其整体设计是否良好？
  3. 然后按合理顺序审查 CL 的其他内容。

第一步：全面了解变化

先查看 CL 的描述和整体修改内容，确保这项变更本身是否有意义，如果根本不需要，请立即回复并说明否决理由。在拒绝此类变更时，最好能同步给开发者替代方案的建议。

比如，你可以这样回应：『看起来你已经完成一些不错的工作，谢谢！但实际上，我们当前正在逐步淘汰你所修改的 FooWidget 系统，因此现阶段不再接受对其的新修改。不过，你来重构下新的 BarWidget 类怎么样？』

请注意，评审人不仅拒绝了当前 CL 并提供了替代方案，更始终保持着专业和礼貌。这种相互尊重的态度很重要，即便存在分歧，开发者之间也要彼此尊重。

如果频繁收到不符合预期的变更请求，建议重新审视团队的开发流程或对外贡献者指南，确保在代码编写前进行更充分的沟通。与其让他人付出大量心血后被迫废弃或重写，不如尽早明确方向以避免资源浪费。

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

定位本次 CL 的主要文件。通常存在一个包含最多逻辑修改的主文件，并且它是 CL 的主要部分。优先审查这些核心内容有助于理解所有辅助性修改的上下文，从而提升代码评审效率。若因 CL 规模过大难以确定核心部分，请直接请开发者指明优先评审范围，或要求将 CL 拆分为多个独立的变更集。

如果 CL 的核心部分有重大的设计问题，要立即提出反馈，即使还没有时间完成全面的评审。因为如果对方的设计有严重问题，那继续审查当前 CL 的其他部分就是浪费时间，该作者后续的 CL 也变得无关紧要了。

立即提交重大设计意见至关重要，主要有两个原因：

  1. 开发者提交 CL 后，往往会在评审期间立即基于该变更开展后续工作。若核心设计存在缺陷，其后续工作也将被迫返工。必须在其基于问题设计投入过多额外工作前及时制止。
  2. 重大设计更改比小更改需要更长的时间。几乎所有的开发者都有最后期限；为了在截止日期前完成这些任务，还要保持高质量代码，开发者就需要尽快对 CL 进行返工。

第三步：按适当的顺序浏览 CL 的其余部分

一旦你确认整个 CL 没有重大设计问题，尝试找出一个逻辑顺序来查看文件，同时确保你不会错过查看任何文件。通常，在浏览完主要文件后，最简单的方法是按照代码审查工具向你显示它们的顺序浏览每个文件。有时，在看代码之前先看测试也很有帮助，因为这样你就会知道更改做了什么。

Code Review 的速度

为什么 Code Review 应该很快？

在 Google，我们针对团队共同制作产品的速度进行优化， 而不是针对单个开发者编写代码的速度进行优化。个人发展的速度很重要，但它并不如整个团队的速度那么重要。

当 Code Review 太慢时，会发生以下几种情况：

  • 整个团队的速度降低。 不快速回复评论的人的确会完成其他工作。但是，团队其他成员的新功能和错误修复会延迟数天、数周或数月，只因为每个 CL 都在等待审查和重新审查。
  • 开发者会开始对 Code Review 流程产生抵触情绪。 如果审查者每隔几天才回复一次，并且每次都要求对 CL 进行重大更改，这种工作节奏极易让开发者产生挫败感。这种情绪往往表现为抱怨评审者『过于严苛』。但倘若评审者虽然坚持要求实质性改进（这些改进确实能提升代码健康度），却在开发者每次更新后都能快速响应，此类抱怨通常会自然消散。事实上，通过加速评审流程本身，能解决大多数关于代码评审的投诉。
  • 代码健康度可能受到侵蚀。 当评审进程缓慢时，允许开发者提交不如预期的 CL 的压力就会增加。缓慢的审查还阻碍了代码清理、重构和对现有 CL 的进一步改进。

Code Review 应该多快？

如果你不在执行很重要的任务，那么应该在收到代码审查后尽快开始。

一个工作日是响应 Code Review 请求所需的最长时间 （即第二天早上的第一件事）。

遵循这些指南意味着典型的 CL 应该在一天内进行多轮审查（如果需要）。

速度 VS 中断

在个人效率与团队效率的权衡中，存在一个特殊例外：当你正处于深度专注的工作状态时（例如编写代码），不应中断当前任务进行代码评审。 研究表明，开发者被打断后需要很长时间才能重新进入心流的开发状态。因此，中途自我中断进行 Code Review 对团队造成的实际损失，反而比让另一位开发者稍候评审更大。

正确的做法是在工作出现自然中断点时处理评审请求。例如完成当前编码任务后、午餐后、会议结束后、从休息区返回时等节点。

快速响应

在讨论代码评审速度时，我们关注的核心是响应时效，而不是整个评审流程的完成时间。虽然完整的评审流程也应当高效推进，但相比整体速度，单个评审响应的及时性更为重要。

即使整个评审周期较长，只要评审者能持续保持及时响应，就能显著缓解开发者对"评审缓慢"的挫败感。

如果你太忙了没有时间对一个 CL做一次完整的 Review，你也可以快速回复下，让开发者知道你什么时候可以开始、推荐能更快响应的其他评审人、或提供初步的整体意见。（注意：即使发送此类通知也不应中断编码工作——在工作中的自然中断点发送响应。）

重要的是，审查者要花足够的时间确保他们的『LGTM』真正代表『代码符合质量标准』。但是，在此基础上还是应该追求快速响应的专业习惯。

跨时区 Review

在处理跨时区协作时，尽量在作者当日工作时间结束前完成评审反馈，确保其有充足时间回应。若对方工作日已结束，那么请确保在他们第二天开始工作之前完成您的审查。

LGTM 和评论

为了加快 Code Review 速度，在某些情况下，审查者直接给出『LGTM / Approval』结论，即使 CL 中有未解决的注释。适用情形包括（满足至少一项即可）：

  • 审阅者相信开发者将适当地处理审阅者的所有剩余评论。
  • 提出的意见不需要开发者强制修改。
  • 很小的优化建议，比如：调整导入顺序、修复附近的拼写错误、应用建议的修复、删除未使用的依赖等。

若适用情形不够明确，评审人应具体说明所依据的条款。

当开发者和审查者不在同一时区时，LGTM 的评论尤其重要，否则开发者需要等待一整天才能获得『LGTM / Approval』。

大型 CL

如果有人向你发送的 CL 太大了，你也不确定何时有时间查看，那么你应该要求开发者拆分为多个相互承接的小型提交，而不是一次性提交一个必须整体评审的巨大 CL。即使会增加开发者工作量，但这种拆分通常可行且能显著提升评审效率。

如果确实无法拆分提交，且审查者无法快速完成全面评审，至少应对整体设计架构提出建设性意见并返还给开发者改进。作为审阅者，你的目标之一应该是在保障代码健康度的前提下，始终为开发者扫清障碍，确保其能持续快速推进工作。

Code Review 随时间推移而改进

如果你遵循这些准则并且严格要求 Code Review，你应该会发现随着时间的推移，整个 Code Review 过程往往会越来越快。开发者逐渐掌握健康代码的要求，提交的 CL 从一开始就更加完善，所需评审时间持续减少。审查者则学会快速响应，避免在流程中引入不必要的延迟。但千万不要为了追求表面效率而降低评审标准或质量——从长远来看，这并不会真正加速开发进程。

紧急情况

还有一些紧急情况，CL 必须非常快速地通过整个审查流程，并且质量准则将放宽。请查看什么是紧急情况？ 中描述的哪些情况属于紧急情况，哪些情况不属于紧急情况。

Code Review 的评论

总结

  • 保持善良。
  • 解释你的评论。
  • 在提供明确指示与仅指出问题并让开发者自行决定之间保持平衡。
  • 鼓励开发者简化代码或添加代码注释，而不仅仅是向你解释复杂性。

礼貌

一般而言，在保持清晰和乐于助人的同时，对接受代码审查的开发者始终保持礼貌与尊重至关重要。一种方式是确保你的评论始终针对代码，而不是针对开发者本人。你不需要总是遵循这种做法，但在可能引发争议或令人不快的评论中务必遵循该方式。比如：

Bad：『这里明明用线程没有任何好处，为什么还要用多线程？』

Good：『此处的并发模型增加了系统复杂性，也没有带来任何实际可见的性能提升。因为没有性能收益，建议将此代码改为单线程模式而不是使用多线程。』

解释原因

你会注意到，上面『Good』的例子有一个特点：它帮助开发者理解你提出建议的原因。虽然不需要在每个审查评论中都写这样的解释，但适时说明你的意图、遵循的最佳实践或者建议如何提升代码健康度，往往能取得更好效果。

提供指导

通常而言，修改代码是开发者的职责，而不是审查者的义务。 你不需要为开发者详细设计解决方案或直接编写代码。

但这并不意味着审查者应该袖手旁观。关键在于在指出问题与提供直接指导之间找到平衡点。指出问题并让开发者做出决定通常有助于他们学习成长，同时降低代码审查的阻力。由于开发者比审查者更了解代码细节，这种方式还可能催生更优秀的解决方案。

但是，有时，明确告诉他们怎么做，甚至直接告诉他们代码怎么改会更有帮助。Code Review 的首要目标是得到尽可能好的 CL。次要目标则是通过持续提升开发者技能，使其逐渐减少对审查的依赖。

请记住，人们不仅从改进建议中学习，更需要通过正向反馈获得激励。如果你在 CL 中发现值得赞赏之处，比如：开发者优化了混乱的算法、增加了完善的测试覆盖，或是你作为审查者从中有所收获，请毫不吝啬地给予赞美。与所有评论一样，请具体说明欣赏的原因，这将进一步鼓励开发者保持良好实践。

标注评审意见的严重级别

建议为评审意见标注严重程度级别，区分必须修改项与指导性建议。

举例说明：

Nit（Nitpick）：琐碎项，技术细节问题。虽然建议修正，但不会产生重大影响。

Optional / Consider：可选项，改进建议，非强制要求但值得考虑。

FYI：参考项，不需要在本次 CL 中实施，但可作为未来优化的思路参考。

通过分级标注可使评审意图更加明确，帮助开发者合理分配处理优先级，同时减少沟通误解。例如，如果没有注释标签，开发者可能将所有意见均视为强制要求，而实际上部分意见仅具参考价值。

接受解释

如果你要求开发者解释一段你不理解的代码，这往往会让他们重构代码以提升清晰度。有时，在代码中添加注释也是恰当的解决方式，前提是该注释不能仅仅是为了解释过度复杂的代码而存在。

仅在代码审查工具中评论解释对未来的代码阅读者毫无帮助。 这种解释仅在少数情况下可被接受，例如当你审查不太熟悉的代码区域时，开发者对普通代码阅读者本应知晓的内容进行了说明。

如何处理审查者的反对

有时开发者会对 Code Review 提出反对。他们可能不认同你的建议，或者抱怨审查标准过于严格。

谁是对的？

当开发者不同意你的建议时，首先应冷静思考他们的观点是否合理。通常说，开发者比审查者更贴近代码细节，因此可能对某些方面有更深入的见解。他们的论点是否站得住脚？从代码健康度的角度来看是否合理？若确实如此，应坦然认可其正确性并停止争议。

但是，开发者也并不总是正确的。此时审查者需要进一步解释建议的合理性。优秀的解释应包含两个要素：表明理解开发者的回复，同时提供要求变更的额外依据。

特别是，当审查者认为他们的建议能提升代码健康度，产生的代码质量改进值得投入额外工作时，就应当坚持主张。代码健康度的提升往往源于点滴积累。

有时，一个建议需要多轮解释才能真正被理解。只要确保始终保持礼貌，让开发者知道你听取了他们的意见，只是持有不同观点。

令开发者沮丧

审阅者有时担心，如果坚持要求改进会引发开发者抵触。虽然开发者确实可能产生短暂的不悦，但他们往往会在后续阶段感激你帮忙提升代码质量。实际上，只要措辞保持礼貌，开发者通常并不会真正心生芥蒂，这种忧虑多半存在于审阅者的主观想象中。引发不快的根源往往在于意见表述的方式，而不是对代码质量的坚持。

后续清理

开发者（理所当然地）希望快速完成任务，不愿为当前提交反复进行审阅流程，因此常以『后续提交会清理』为由要求先通过这个 CL。部分开发者确实会立即编写修复补丁，但实践表明：随着原始提交时间推移，清理工作的执行概率呈指数级衰减。除非能当场完成清理，否则这些改进很可能永无实现之日。这并非因为开发者不负责任，而是繁重的工作压力导致改进任务被淹没或遗忘。因此最稳妥的做法是坚持要求即时改进，而不是放任代码入库后再修补，『后续清理』往往是代码质量劣化的开端。

如果 CL 引入了新的复杂性，则必须在提交之前对其进行清理，除非是紧急情况。如果 CL 暴露了周围的问题并且现在无法立即解决，开发者应该提交一个 bug 进行清理并将其分配给自己，以免丢失。他们还可以选择在引用已提交错误的代码中编写 TODO 注释。

对严格评审的普遍性质疑

如果你以前的 Code Review 相当宽松，并且你改用严格的评论，有些开发者会非常大声地抱怨。改进 Code Review 的速度通常能有效缓解这类抱怨。

有时这些抱怨可能需要几个月才能消散，但最终开发者们往往会看到严格代码审查的价值，因为他们看到了它帮助他们生成了哪些优秀的代码。有时，那些最激烈的抗议者一旦发生了让他们真正看到你通过严格审查所增加的价值的事情，甚至可能成为你最坚定的支持者。

解决分歧

如果你遵循了上述准则后仍与开发者存在不可调和的分歧，请参考Code Review 的标准中列出的指导原则来化解矛盾。

代码变更者指南

编写好的 CL 描述

CL 描述是更改的公共记录，重要的是它传达：

  1. What：正在进行哪些变更？应该要总结主要变化，以便读者无需阅读整个 CL 即可了解正在更改的内容。
  2. Why：为什么要进行这些更改？作为作者，在进行此变更时，你有什么背景？你是否做出了一些未反映在源代码中的决定？等。

CL 描述将成为我们版本控制历史的永久组成部分，未来可能被数百人查阅。

未来的开发者将依据描述内容检索你的变更。当有人仅模糊记得某个相关修改却缺乏具体细节时，如果所有重要信息都在代码中而不是描述中，那么就很难定位到你的 CL。

然后，在他们找到 CL 后，他们是否能够理解为什么做这样的变更？阅读源码可以知道做了什么，但是很难解释为什么这么做，这就可能会让未来的开发者很判断是否可以移动『切斯特顿栅栏』。

写得好的 CL 描述将帮助那些未来的工程师——有时，包括你自己！

第一行

  • 正在做的事情的简短摘要。
  • 完整的句子，像命令一样。
  • 后面是空行。

CL 的描述首行应该是对本次变更内容的简要总结，后面跟一个空行。这是版本控制历史摘要中出现的内容，因此它应该提供足够的信息，让未来的代码搜索者不必阅读你的 CL 或其整个描述即可理解你的 CL 实际做了什么或它与其他 CL 有何不同。也就是说，首行应自成一体，让读者能更高效地浏览代码历史。

尽量保持第一行简短、重点突出、切中要害。清晰度和对读者的实用性应作为首要考量。

按照惯例，CL 描述的第一行是一个完整的句子，写起来就好像它是一个命令（祈使语气）。比如『删除 FizzBuzz RPC 并用新系统替换它』，而不是『正在删除 FizzBuzz RPC 并用新系统替换它』。不过，你不必将其他部分也写成祈使句。

主体应该信息丰富

第一行应该是一个简短、重点突出的摘要，而其他的描述应该填写细节，包含读者全面理解变更所需的辅助信息。它可能包括：待解决问题的简要说明、采用当前方案的理由。如果该方法有任何缺点，也要明确说明。也可包含相关的背景信息，比如错误编号、基准测试结果、设计文档链接等。

如果包含外部资源链接，请注意未来读者可能因访问限制或留存政策无法查看。应该尽可能提供足够的背景信息，确保审查者和未来读者能理解 CL 内容。

即便是小型 CL 也值得注重细节。需要说明 CL 的上下文背景。

不好的 CL 描述

『修复 BUG』是不充分的 CL 描述。什么 BUG？你做了什么来修复它？其他类似糟糕的描述包括：

  • 『修复构建。』
  • 『添加补丁。』
  • 『将代码从 A 移动到 B。』
  • 『增加了便利功能。』
  • 『删除奇怪的 URLs。』

其中一些是真实的 CL 描述。虽然简短，但它们没有提供足够的有用信息。

良好的 CL 描述

以下是一些良好的 CL 描述示例。

功能变更型

举例：

RPC：删除 RPC 服务器消息空闲列表的大小限制。

像 FizzBuzz 这样的服务器需要处理超大规模消息，扩大空闲列表将显著提升消息复用效率。本次变更不仅扩大了空闲列表容量，还新增了 goroutine 用于渐进式释放列表条目，确保空闲服务器最终能释放所有闲置资源。

前几个词就描述了 CL 的实际工作。后续描述依次说明：待解决的问题、采用此方案的优势，以及具体实现细节的补充信息。

代码重构型

举例：

通过注入 TimeKeeper 实例构建 Task 对象以调用其 TimeStr 和 Now 方法。

在 Task 类中新增 Now 方法，以便可以删除 borglet() getter 方法（仅让 OOMCandidate 用于调用 borglet 的 Now 方法）。这取代了 Borglet 上委托给 TimeKeeper 的方法。

允许 Task 提供 Now 方法是解耦 Borglet 依赖的重要一步。最终，所有从 Task 获取 Now 方法的协作模块都应改为直接使用 TimeKeeper，但本次变更采用了渐进式重构策略。

继续重构 Borglet 层次结构应作为长期目标。

第一行描述了 CL 的作用以及与旧版本的差异，后续描述涵盖：具体实现方案、变更背景、当前方案的局限性、未来优化方向，并阐释了本次变更的意义。

需要上下文的小型 CL

举例：

为 status.py 创建 Python3 的构建规则。

现有 Python3 用户可以直接依赖原始的状态构建规则旁边的配套规则，而不需要在自己的规则树中维护。它鼓励新的消费者尽可能使用 Python3 而不是 Python2，同时极大简化了当前正在开发的自动化构建文件重构工具。

第一句描述了实际正在做的事情。其他部分解释了变更的原因，并为审查者提供了充分的上下文。

使用标签

标签是手动输入的标识符，可用于对 CL 进行分类。这些标签可通过工具支持，或者仅作为团队约定使用。

比如：

  • [tag]
  • [a longer tag]
  • #tag
  • tag:

使用标签为可选操作。

添加标签时，需考虑应将其置于 CL 描述的正文还是首行。应限制在首行使用标签的频率，以免影响核心内容的呈现。

带标签和无标签示例：

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16

// 如果保持简洁，标签可以放在首行。
[banana] Peel the banana before eating.

// 标签可以内嵌在内容中。
Peel the #banana before eating.

// 标签是可选的。
Peel the banana before eating.

// 如果保持简短，多个标签是可以接受的。
#banana #apple: Assemble a fruit basket.

// 标签可以放在 CL 描述的任何地方。
> Assemble a fruit basket.
>
> #banana #apple

1
2
3

// 标签过多（或标签过长）会占据第一行。
// 相反，考虑是否可以将标签移动到描述正文中，或缩短它们。
[banana peeler factory factory][apple picking service] Assemble a fruit basket.

生成的 CL 描述

一些 CL 是由工具生成的。只要可以，他们的描述也应该遵循此处的建议。也就是说，它们的第一行应该简短、重点突出且独立，并且 CL 描述正文应包含信息丰富的细节，以帮助审阅者和未来的代码搜索者理解每个 CL 的效果。

提交前检查

CL 在审查期间可能会发生重大变化。在提交 CL 之前查看 CL 描述，确保 CL 的描述正确反映了变更内容。

小型 CL

为什么要写小型 CL？

小型、简单的 CL 有以下优势：

  • 审查速度更快。 对于审查者来说，多次抽出 5分钟来审查小型 CL 比留出 30分钟的时间来审查一个大型 CL 要容易得多。
  • 审查得更彻底。 大型变更会让审查者和作者因为大量细节反馈然后反复沟通而感到沮丧——甚至可能导致重要问题被遗漏或忽视。
  • 减少缺陷引入。 由于改动量小，你和审查者能更清晰地评估变更影响，更容易发现潜在 BUG。
  • 避免无效劳动。 如果你写了一个巨大的 CL，然后你的审查者说整体方向是错误的，前期投入将大量浪费。
  • 更容易被合并。 处理大型 CL 需要很长时间，因此合并时会遇到很多冲突，并且必须经常合并。
  • 更容易设计好。 小变更能更精细地打磨设计与代码健康度，而大变更的细节优化难度更高。
  • 减少审核阻塞。 将大变更拆分为独立部分后，你可以在等待审核时继续其他编码工作。
  • 回滚更方便。 大型 CL 涉及的文件可能在初始提交与回滚期间被更新，导致需要连环回滚（涉及中间版本），大幅增加回滚复杂度。

请注意，审查者有权仅因变更规模过大而直接拒绝合入。 虽然通常会感谢贡献，但会要求将其拆分为系列小变更。事后再拆分可能需要大量工作，或者需要大量时间说服审查者应该接受你的大更改。不如一开始就编写小型的 CL 更容易。

什么是小型 CL？

一般来说，CL 的正确大小是一个独立的变更。这意味着：

  • CL 只做了一件事的最小更改。通常只是功能的一部分，而不是一个完整的功能。一般来说，相较于过大变更，更推荐编写偏小的变更，具体尺度可与审查者共同商定。
  • CL 应该包含相关的测试代码。
  • 审查者所需的所有信息（除未来开发计划外）应通过 CL 本身、变更描述、现有代码库或已审阅过的 CL 就能获取。
  • CL 提交后需确保用户端系统稳定运行，且不影响开发人员后续工作。
  • CL 不宜过小而导致逻辑难以理解。如果添加新的 API，则应在同一 CL 中包含该 API 的用法，以便审查者能够更好地了解该 API 的使用方式。也避免提交未被使用的 API。

关于『太大』没有硬性规定。对于 CL 来说，100行通常是合理的大小，而 1000 行通常太大了，但这取决于你的审查者的判断。变更涉及的文件数量也会影响感知规模『大小』。一个文件中的 200 行更改可能没问题，但分布在 50 个文件中通常太大。

请记住，作为代码创作者你全程参与开发，但审查者通常缺乏上下文背景。你认为适中的 CL 大小可能会让审查者不堪重负。如有疑问，应编写比你预期更小的 CL。审查者很少会抱怨 CL 太小。

什么时候可以提交大型 CL？

在下面几种情况下，大型 CL 可能不会那么糟糕：

  • 你通常可以将删除整个文件看做一行变更，因为审查者不会花很长时间来审查。
  • 有时，一个大型 CL 是由你完全信任的自动化重构工具生成的，而审查者的工作只是验证并他们确实想要的变更。这些 CL 可适当扩大规模，但仍需注意前文提到的合并冲突、测试验证等注意事项。

高效编写小型 CL

如果每次提交小型 CL 都需要等待审核通过才继续开发，那会浪费很多时间。因此，建议采用这些策略避免工作阻塞：同时处理多个项目、寻找可以立即开始的审查者、面对面审查、结对编程或拆分你的 CL 使后续开发不受阻。

拆分 CL

在开始一项涉及多个有潜在依赖关系的 CL 变更时，先宏观规划如何拆分和组织这些 CL 再开始编码，可能会更有用。

这种做法不仅方便作者更容易管理和组织 CL，也能让审查者更轻松地理解修改内容，从而提升代码审查效率。

以下是将工作拆分为不同 CL 的一些策略。

变更堆叠法

一种不影响开发进度的拆分方法是：先完成一个小型变更，提交审查后立即基于该变更开始编写下一个。大多数版本控制系统都支持这种操作方式。

按文件拆分

另一种方法是按文件分组，当不同文件需要不同审查者评审，且每个变更集本身具有完整性和独立性时适用。

比如：你可以将协议缓冲区的修改作为一个 CL，而将该协议的使用代码修改作为另一个 CL。虽然协议变更必须先于代码变更提交，但两者可以同步进行审查。采用这种方式时，建议同时告知两组审查者另一个 CL 的存在，以便他们全面理解修改背景。

另一个例子是：你发送了一个代码变更的 CL，然后另一个变更是使用该代码的配置或实验文件。这种方式还便于必要时回滚操作，因为配置/实验文件有时比代码变更能更快推送到生产环境。

水平拆分

我们可以考虑创建共享代码或打桩（stubs），让一个程序的不同部分（比如前端界面和后端服务）能够先各干各的，互不干扰。这不仅能加快开发进度，还能促进各层之间的抽象化。

比如：你创建了一个包含客户端、API、服务层和数据模型层的计算器应用程序。通过共享的协议缓冲区（proto）定义接口，可以实现服务层与数据模型层的相互抽象。同样，API 插桩可以将客户端代码与服务代码的实现分离，使两者能够独立推进。类似的思路也可应用于更细粒度的函数或类级别抽象。

垂直拆分

与水平拆分方法正交的是，你可以将代码拆分为更小的、全栈式的纵向功能单元。每个单元可作为独立的并行实现路径，使得某些任务能够持续推进，而其他任务可等待审查或反馈。

回到水平拆分中的计算器示例：现在你需要支持乘法和除法等新运算符。你可以将乘法和除法作为独立的垂直子功能来拆分实现，尽管它们可能存在部分重叠（例如共享的按钮样式或验证逻辑）。

水平和垂直拆分

更进一步，你可以组合运用这些方法，绘制如下实施规划图，其中每个单元格都是独立的 CL 单元。从底层模型开始向上推进至客户端：

层级功能：乘法功能：除法
客户端添加按钮添加按钮
API 层添加 API 端点添加 API 端点
服务端实现乘法转换逻辑共享转换逻辑（基于乘法实现扩展）
模型层添加乘法协议定义添加除法协议定义

分离重构

重构工作最好与功能变更或缺陷修复放在不同的 CL 中完成。比如：移动和重命名类的操作应该与修复该类中的缺陷分别放在两个独立的CL中。这样做能使审查者更清晰地理解每个 CL 所引入的变更。

不过，小的清理工作（比如修复局部变量名）可以包含在功能变更或缺陷修复的 CL 中。具体何时需要将重构拆分出来，取决于开发者和审查者的判断——当重构规模过大导致当前 CL 审查难度增加时，就应考虑进行拆分。

CL 应该包含相关的测试代码。需要注意的是，此处『小型』指的是变更内容在概念上的聚焦程度，而不是单纯依据代码行数进行衡量。

在谷歌的所有代码变更中都要求包含测试。

对于添加或修改逻辑的变更，必须配备针对新行为的新增或更新测试。纯重构类变更（不意图改变行为）同样需要测试覆盖；理想情况下这些测试应已存在，若尚未覆盖则应补充测试。

独立的测试修改可优先放在单独的 CL 中，这与重构准则类似，具体包括：

  • 通过新增测试验证已提交的既有代码。
    • 确保测试涵盖重要逻辑。
    • 提高对受影响代码的后续重构的信心。比如，若需重构未被测试覆盖的代码，应先提交测试 CL 来验证重构前后行为一致性。
  • 重构测试代码（比如引入辅助函数）。
  • 引入大型测试框架代码（比如集成测试）。

不要破坏构建

若存在多个相互依赖的 CL，必须确保每个 CL 提交后整个系统仍能正常运行。否则，在连续提交多个 CL 的间隙（尤其是后续 CL 意外出错时），可能导致其他开发者的构建流程中断数分钟甚至更长时间。

不能让它足够小

有时你可能会遇到必须使用大型 CL 的情况。但实际上这种情况极少出现。习惯编写小型 CL 的开发者几乎总能找到将功能分解为系列小型变更的方法。

在编写大型 CL 之前，请考虑在它前面加上一个仅重构的 CL 来为更简洁的实现铺平道路。与团队成员交流，看看是否有人有思路通过小型 CL 实现该功能。

如果这些方案都无效（这种情况比较少），需要提前获得审查者对大型 CL 的评审许可，让他对后续的工作有预期。在这种情况下，做好延长评审周期的准备，高度警惕避免引入缺陷，并应该特别注重编写测试用例。

如何处理审查者的评论

当你将 CL 提交后，你的审查者很可能会针对你的 CL 评论几条。以下是关于如何处理评审者评论的一些有用建议。

不要把它当时针对个人的

审查的目标是保持我们代码库和产品的质量。当审查者对你的代码提出批评时，请将其视为他们尝试帮助你、代码库和 Google，而不是对你或你能力的人身攻击。

有时审查者可能会在评论中流露出挫败感。这不是很专业的行为，但作为开发者，你应该有心里准备。问问自己：『审查者试图传达的建设性意见是什么？』，基于这个角度理解他们的意见。

永远不要愤怒地回应代码审查评论。 这不仅严重违背职业准则，这些不当回应还会永久保留在代码审查工具中。如果你太生气难以理性回复，可以先暂时离开工作区或处理其他任务，待平静后再礼貌回复。

一般来说，如果审查者没有以建设性和礼貌的方式提供反馈，建议亲自向他们解释。如果你无法亲自或通过视频通话与他们交谈，请向他们发送私人电子邮件。以友善的方式说明你希望对方改进的沟通方式。若私下沟通仍无效或对方反应依然不专业，可以适时向你的经理升级该情况。

修复代码

如果审查者说他们不理解代码中的某些内容，你的第一个反应应该是澄清代码本身。如果无法澄清，请考虑添加注释来解释代码存在的原因。当添加注释确实不适用时，可以考虑在代码审查工具中进行文字解释。

审查者难以理解的代码，未来的其他阅读者同样可能无法理解。在审查工具中的文字解释无法帮助未来的代码阅读者，而优化代码或添加注释却能真正帮助他们。

协作思考

编写 CL 往往需要投入大量精力。当最终提交审查时，开发者常会产生『已完成且无需修改』的满足感。此时若收到修改要求，尤其是存在异议的反馈，容易产生挫败感。

此时请退后一步思考：审查者是否提供了对代码库和公司有价值的建议？首先应自问：『我是否真正理解了审查者的要求？』

若存在疑惑，应主动请求审查者澄清。

若你理解意见但持有不同观点时，请以协作而非对抗或防御的心态进行沟通：

1

错误师范：『不，我拒绝修改』

1

正确示范：『我选择 X 方案是基于[这些优缺点]及[这些权衡]。 据我分析 Y 方案会因[具体原因]导致效果不佳。你建议采用 Y 方案是认为它能更好地满足原始权衡标准，还是认为我们应该调整权衡权重，或有其他考量？』

记住，礼貌与尊重始终应该作为首要原则。 如果你不同意审查者的观点，可以通过需求澄清、讨论利弊，解释方案为何更有利于代码库/用户/公司等方式实现协作。

有时，你可能知道一些审阅者不知道不了解的用户需求、代码库背景或变更细节。应在适当修改代码的同时，通过提供更多背景信息与审查者展开讨论。通常，你可以基于技术事实的沟通能帮助双方达成共识。

解决分歧

解决分歧的第一步应该始终是尝试与你的审查者达成共识。如果无法达成共识，请参阅代码审查标准，给出了在这种情况下要遵循的原则。

紧急情况

有时，紧急 CL 必须尽快完成整个 Code Review 流程。

什么是紧急情况？

紧急 CL 将是一个小的变更，它允许主要发布继续而不是回滚，修复在生产中对用户产生重大影响的 BUG，处理紧急的法律问题，堵住一个重大的安全漏洞，等等。

在紧急情况下，我们确实关心整个 Code Review 的速度，而不仅仅是响应速度。在这种情况下，审查者应该更关心审查的速度和代码的正确性（真的解决了紧急情况吗？）。当然（很明显），当紧急情况出现时，此类审查应该是最高优先级。

但是，在紧急情况解决后，你应该再次查看紧急 CL 并对其进行更彻底的审查。

什么不是紧急情况？

需要明确的是，以下情况不属于紧急情况：

  • 希望本周而不是下周发布（除非存在硬性截止时间，如合作伙伴的协议约定）。
  • 开发者长期开发某个功能后迫切希望提交合并 CL。
  • 审查者都处在不同时区，且正值深夜或者不在现场。
  • 周五快下班时，开发者希望赶在周末前提交 CL。
  • 管理者以软性截止日期（非硬性）为由，要求今天必须完成评审并合并 CL。
  • 因导致测试失败或构建中断而回滚 CL。

等等。

什么是硬性截止日期？

硬性截止日期指如果你错过了它，就会发生灾难性的事情。比如：

  • 根据合同约定必须在特定日期前提交 CL。
  • 产品若未在指定日期前发布将彻底失去市场竞争力。
  • 某些硬件制造商每年仅进行一次新硬件部署，如果错过代码提交期限（取决于代码类型）可能造成灾难性影响。

推迟一周发布通常不会造成灾难性后果。错过重要的会议可能是灾难性的，但多数情况下并不是。

绝大多数截止期限都属于软性期限，而非硬性期限。这些期限代表了对特定功能按时完成的期望，虽然重要，但不应以牺牲代码健康度为代价来满足。

如果你的发布周期很长（几周），开发者可能为赶在下一周期前完成功能而降低 Code Review 标准。然而，如果重复这种模式，项目会慢慢积累沉重的技术债务。如果开发者经常在周期接近尾声时提交『必须合入』的 CL，而这些 CL 只是进行表面审查，那么团队就应调整开发流程，以便在周期早期实施重大的功能更改，并留有足够的时间进行良好的审查。