Google 代码审查指南

Google 代码审查指南

简介

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

Google 通过 Code Review 来维护代码和产品质量。

此文档是 Google Code Review 流程和政策的规范说明。

Google Code Review有两套文档：

• 代码审查者指南：针对代码审查者的详细指南。
• 代码开发者指南：针对 CL 开发者的的详细指南。

背景

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

术语

文档中使用了 Google 内部术语，在此为外部读者澄清：

• CL：代表“变更列表（changelist）” ，表示已提交到版本控制或正在进行代码审查的自包含更改。有的组织会将其称为“变更（change）”或“补丁（patch）”。
• LGTM：意思是“我觉得不错（Looks Good to Me）” 。这是批准 CL 时代码审查者所说的。

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

代码审查时应该关注以下方面：

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

选择最合适审查者

一般而言，您希望找到能在合理的时间内回复您的评论的最合适的审查者。

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

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

面对面审查

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

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

代码审查者指南

这是基于过往经验编写的 Code Review 最佳方式建议。其中分为了很多独立的部分，共同组成完整的文档。虽然您不必阅读文档，但通读一遍会对您自己和团队很有帮助。

Code Review 标准

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

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

首先，开发人员必须能够对任务进行 改进 。如果开发者从未向代码库提交过代码，那么代码库的改进也就无从谈起。此外，如果审核人员对代码吹毛求疵，那么开发人员以后也很难再做出改进。

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

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

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

一般来说，审核人员应该倾向于批准 CL，只要 CL 确实可以提高系统的整体代码健康状态，即使 CL 并不完美。

这是所有 Code Review 指南中的 高级原则 。

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

此处有一个关键点就是没有“完美”的代码，只有 更好 的代码。审查者不该要求开发者在批准程序前仔细清理、润色 CL 每个角落。相反，审查者应该在变更的重要性与取得进展之间取得平衡。审查者不应该追求完美，而应是追求持续改进。不要因为一个 CL不是“完美的”，就将可以提高系统的可维护性、可读性和可理解性的 CL 延迟数天或数周才批准。

审核者应该随时在可以改善的地方留下审核评论，但如果评论不是很重要，请在评论语句前加上“Nit：”之类的内容，让开发者知道这条评论是用来指出可以润色的地方，而他们可以选择是否忽略。

注意：本文档中没有任何内容证明检查 CL 肯定会使系统的整体代码健康状况恶化。您会做这中事情应该只有在 紧急情况 时。

指导

代码审查具有向开发人员传授语言、框架或通用软件设计原则新内容的重要功能。留下评论可以帮助开发人员学习新东西 ，这总归是很好的。分享知识是随着长年累月改善系统代码健康状况的一部分。请记住， 如果您的评论纯粹是教育性的，且对于本文档中描述的标准并不重要，请在其前面添加“Nit：”或以其他方式表明作者不必在此 CL 中解决它。

原则

• 基于技术事实和数据否决意见和个人偏好。
• 关于代码风格问题， 风格指南 是绝对权威。任何不在风格指南中的纯粹风格点（例如空白等）都是个人偏好的问题。代码风格应该与风格指南中的一致。如果没有以前的风格，请接受作者的风格。
• 软件设计方面几乎不是纯粹的风格或个人偏好问题。软件设计基于基本原则且应该权衡这些原则，而不仅仅是个人意见。有时候会有多种有效的选择。如果作者可以证明（通过数据或基于可靠的工程原理）该方法同样有效，那么审查者应该接受作者的偏好。否则，就要取决于软件设计的标准原则。
• 如果没有其他适用规则，则审查者可以要求作者与当前代码库中的内容保持一致，只要不恶化系统的整体代码健康状况即可。

解决冲突

如果在代码审查过程中有任何冲突，第一步应该始终是开发人员和审查者根据本文档中的 开发者指南和审查者指南 达成共识。

当达成共识变得特别困难时，审阅者和开发者可以进行面对面的会议，或者有 VC 参与调停，而不仅仅是试着通过代码审查评论来解决冲突。 （但是，如果您这样做了，请确保在 CL 的评论中记录讨论结果，以供将来的读者使用。）

如果这样还不能解决问题，那么解决该问题最常用方法是将问题升级。通常是将问题升级为更广泛的团队讨论，有一个 TL 权衡，要求维护人员对代码作出决定，或要求工程经理的帮助。 不要因为 CL 的开发者和审查者不能达成一致，就让 CL 在那里卡壳。

Code Review 要点

注意：在考虑这些要点时，请谨记 “ Code Review 标准 ”。

设计

审查中最重要的是 CL 的 整体设计。CL 中各种代码的交互是否有意义？此变更是属于您的代码库（codebase）还是属于库（library）？它是否与您系统的其他部分很好地集成？现在是添加此功能的好时机吗？

功能

这个 CL 是否符合开发者的意图？开发者的意图对代码的用户是否是好的？ “用户”通常都是最终用户（当他们受到变更影响时）和开发者（将来必须“使用”此代码）。

大多数情况下，我们希望开发者能够很好地测试 CL，以便在审查时代码能够正常工作。但是，作为审查者，仍然应该考虑边缘情况，寻找并发问题，尝试像用户一样思考，并确保您单纯透过阅读方式审查时，代码没有包含任何 bug。

当要检查 CL 的行为会对用户有重大影响时，验证 CL 的变化将变得十分重要。例如 UI 变