谷歌开源内部代码评审规范

代码评审标准

代码评审的主要目的是确保 Google 代码库的总体代码运行情况随着时间的推移而不断改善。代码评审的全部工具和过程都是为此设计的。

为了实现这一点，必须作出一系列的权衡。

首先，开发人员必须可以在其任务上取得进展。若是开发人员从未向代码库提交过改进，那么代码库将永远不会获得改善。另外，若是评审人员不进行任何更改，那么以后开发人员也没有动力进行改进。

另外一方面，评审人员有责任确保每一个 CL（变动列表）的质量，使得其代码库的总体代码运行情况不会随着时间的流逝而减小。这可能很棘手，由于随着时间的推移，代码库一般会因为代码运行情况的小幅降低而退化，尤为是在开发团队时间紧迫，认为必须采起捷径才能实现其目标的状况下。

此外，评审人员对他们正在评审的代码要负起责任。确保代码库保持一致性和可维护性，以及“代码评审中的查找内容”中提到的其余事项 。

所以，咱们将如下规则做为代码评审中指望的标准：

一般，即便 CL 并不完美，不过若是达到能够提高系统总体代码质量的程度，评审人员就能够批准它。

这是全部代码评审指南中的最高原则。

固然，是有例外的。例如，若是 CL 添加了评审人员不但愿在其系统中使用的功能，那么即便代码设计得当，评审人员也能够确定的拒绝批准。

这里的关键点是，没有“完美”的代码，只有更好的代码。评审人员不该要求开发人员在批准前抛光 CL 的每一小块细节。评审人员要追求的不该该是完美，而应该是持续改进。整体而言，若是一个 CL 能够提升系统的可维护性，可读性和可理解性，那就不该该由于它不是“完美的”而被延迟几天甚至几周。

评审人员应该随时提供建议，这样开发人员可能会作得更好，可是若是不是很重要的建议，能够在其前面加上“ Nit：”之类的字眼，以使开发人员知道这只是一个改进建议，他们能够选择忽略。

指导

代码评审具备重要的功能，能够传授开发人员关于语言，框架或通用软件设计原理的新知识。随着时间的推移共享知识会成为改善系统代码运行质量的一部分。但要注意，若是你的建议纯粹是带有教育性质的，而且对于知足本文所描述的标准来讲并非那么重要，那么请在前面加上“Nit:”，或者以其余方式告诉开发人员，他们并不必定要在 CL 中解决这些问题。

原则

• 客观的技术和数据比我的意见和偏好更重要。
• 在代码风格方面，样式指南是绝对权威。不在样式指南中的任何纯样式点（空格等）都是我的喜爱问题。样式应与那里的样式保持一致。若是没有之前的样式，请接受做者的样式。
• 软件设计方面历来都不是纯粹的样式问题，也不是我的喜爱。它们基于基本原则，应权衡这些原则，而不只仅是我的意见。有时有一些有效的选择。若是开发 人员能够证实（经过数据或基于可靠的工程原理）几种方法一样有效，那么评审人员应接受开发人员的选择。不然就应该基于软件设计标准原则作出决定。
• 若是没有其余的适用规则，那么评审人员能够要求开发人员与当前代码库中的内容保持一致，只要不会使系统的整体代码运行情况恶化就能够。

解决冲突

在代码评审中发生冲突时，第一步应始终是使开发人员和评审人员根据本档以及《 CL 做者指南》 和《审阅者指南》中其余文档的内容达成共识。

当达成共识变得特别困难时，让开发人员和评审人员进行面对面的会议或 VC 可能会有所帮助，而不只仅是尝试经过代码评审注释来解决冲突。（可是，若是这样作，请确保将讨论的结果记录在 CL 上的注释中，以备未来阅读。）

若是还不能解决问题，那么就要考虑把问题升级。升级的途径一般是进行更普遍的团队讨论，其中包括让团队负责人参与进来，请求代码维护人员做出决定，或请求工程经理提供帮助。不要由于开发人员和评审人员没法达成一致意见就让 CL 一直挂在那里。

代码评审中的查找内容

设计

代码评审中最重要的部分是 CL 的整体设计。CL 中各类代码的交互是否有意义？此更改是属于代码库仍是属于某个包？它与系统的其他部分是否集成良好？如今是添加此功能的好时机吗？

功能性

此 CL 是否达到开发人员的预期目的？开发人员打算为该代码的用户带来什么好处？“用户”一般既是最终用户（当他们受到更改影响时）又是开发人员（未来他们将不得不“使用”此代码）。

一般，咱们但愿开发人员可以对 CL 进行良好的测试，以确保它们在进行代码审查时可以正常工做。可是，做为评审人员，仍然应该考虑边缘状况，寻找并发性问题，尝试像用户同样思考，并找出仅仅经过阅读代码不能看到的错误。

您能够根据须要验证 CL，对于评审人员来讲，检查 CL 的行为最好的时机是当它对用户产生了影响，例如 UI 更改。当只是阅读代码的时候，很难理解一些更改将如何影响用户。对于这样的更改，若是过于麻烦而没法在 CL 中打补丁并本身尝试，则可让开发人员提供功能演示。

在代码评审期间考虑功能性的另外一个时机点是，CL 中是否正在进行某种并行编程，从理论上讲可能致使死锁或竞争条件。仅经过运行代码很难检测到这类问题，一般须要有人（开发人员和评审人员）仔细考虑它们，以确保不会引入问题。

复杂性

CL 是否比实际须要的要复杂？在 CL 的每一个级别都进行检查 —— 个别行是否太复杂？功能太复杂？类太复杂？“过于复杂”一般意味着“代码阅读器没法快速理解。”也可能意味着“开发人员在尝试调用或修改此代码时可能会引入错误。”

过分设计一种特殊的复杂性，即开发人员使代码变得比实际须要的通用，或者增长了系统目前不须要的功能。评审人员应特别警戒过分设计。鼓励开发人员解决他们如今须要解决的已知问题，而不是解决开发人员推测的未来可能须要解决的问题。将来的问题应该在它们出现以后再去解决，由于到了那个时候咱们能够看到它们的实际情况和需求。

测验

根据更改要求进行单元测试，集成测试或端到端测试。一般，除非 CL 处理紧急状况，不然应在与生产代码相同的 CL 中添加测试 。

确保 CL 中的测试正确，合理且有用。测试没法自我测试，咱们不多为测试编写测试，因此必须确保测试有效。

代码破损时，测试会失败吗？若是代码在其下面更改，它们将开始产生误报吗？每一个测试都会作出简单而有用的断言吗？测试是否在不一样的测试方法之间适当地分开？

请记住，测试也是必须维护的代码。

命名

开发人员是否使用了良好的命名方式？好的命名要可以充分表达一个项（变量、类名等）是什么或者用来作什么，但又不会由于太长而难以阅读。

注释

开发人员是否用可理解的天然语言写下清晰的注释？全部注释都是必要的吗？一般，好的注释应该解释为何存在某些代码，而不该该解释某些代码在作什么。若是代码不够清晰，没法解释自身，则应使简化代码。也有一些例外状况（例如，正则表达式和复杂算法一般会在注释中解释它们的做用，会让阅读代码的人受益不浅），但大多数注释是针对代码自己可能没法包含的信息，好比这些代码背后的原因。

查看此 CL 以前的注释也会有所帮助。也许有一个待办事项如今能够删除，有意见建议不要进行此更改，等等。

请注意，注释与类，模块或函数的文档不一样，注释应表示一段代码的用途，应如何使用以及使用时的行为。

代码风格

谷歌为主要编程语言和大多数次要编程语言提供了代码风格指南，确保 CL 遵循适当的风格指南。

若是您想改善指南中没有的样式点，请在注释前面加上“ Nit：”，以使开发人员知道这是您认为能够改善代码但不是强制性的选择。但请不要只是基于我的偏好来阻止 CL 的提交。

开发人员不该将主要风格更改与其余更改结合在一块儿。这使得很难看到 CL 中的更改，使合并和回滚更加复杂，并致使其余问题。例如，若是开发人员想要从新格式化整个文件，请他们仅将从新格式化的格式做为一个 CL 发送给评审人员，而后再发送另外一个具备功能更改的 CL。

文档

若是 CL 改变了用户构建，测试，与代码交互或释放代码的方式，请检查其是否还更新了相关文档，包括 README，g3doc 页面和其余生成的参考文档。若是 CL 删除或弃用了代码，请考虑是否还应删除该文档。若是文档缺失，要向开发人员索要。

每一行代码

查看每一行代码。有时能够扫描诸如数据文件，生成的代码或大型数据结构之类的东西，但不要扫描人工编写的类，函数或代码块，并假设其中的内容是能够正常运行的。显然，某些代码比其余代码更须要仔细检查 —— 至因而哪些代码彻底取决于你，但你至少应该要理解这些代码都在作些什么。

若是代码很复杂或者你难以快速看懂它们，这会使评审速度变慢，那么你应该让开发人员知道这一点，并等待他们解释，而后再尝试评审。Google 聘请了出色的软件工程师，若是他们看不懂代码，其余开发人员也极可能看不懂。所以，要求开发人员进行解释，也能够帮助未来的开发人员理解此代码。

若是您理解代码，但又没有资格进行代码评审，请确保在 CL 上有一位合格的评审人员，特别是在复杂性问题（例如安全性，并发性，可访问性，国际化等）上。

上下文

一般，代码查看工具只会显示须要更改部分的几行代码。可是有时必须查看整个文件，以确保更改确实有意义。例如，你可能会看到仅添加了四行代码，可是当你查看整个文件时，您会看到这四行位于 50 行方法中，这个时候须要将其分解为较小的方法。

须要基于整个系统来考量 CL 。此 CL 是在改善系统的代码运行情况，仍是使整个系统更加复杂，更不可测等？不要接受会下降系统代码运行情况的 CL。大多数系统会经过许多小的更改而变得复杂，所以，防止新更改中出现很小的复杂性是很重要的。

好的方面

若是在 CL 中看到不错的东西，请告诉开发人员，尤为是当他们以出色的方式回答了你的评论时。代码评审一般只关注错误，可是也应该鼓励和赞扬良好的实践。有时候告诉开发人员正确的作法比告诉他们错误的作法更有价值。

总结

在进行代码评审时，你应确保：

• 代码通过精心设计。
• 该功能对代码的用户来讲有用。
• UI 的变动合理。
• 任何并行编程都是安全完成的。
• 代码复杂性不要超过应有的程度。
• 不须要实现可能会在将来出现的需求。
• 代码具备适当的单元测试。
• 精心设计的测试用例。
• 开发人员对全部内容都清晰的命名。
• 清晰而有用的代码注释，要解释“为何”，而不是“什么”。
• 恰如其分的代码文档化。
• 该代码符合咱们的风格指南。

确保检查的每一行代码，查看上下文，确保改善代码运行情况，并称赞开发人员所作的出色工做。

检查 CL

摘要

在知道了代码评审要关注哪些东西以后，如何有效地进行跨文件代码评审呢？

• 更改代码有意义吗？它有一个很好的描述吗？
• 首先看一下变化中最重要的部分，总体设计得如何？
• 以适当的顺序查看其他的 CL。

第一步：全面了解代码变动

查看 CL 说明以及 CL 的通常功能。这种变动有意义吗？若是这个变动是没必要要的，请当即作出答复，并说明为何不该该进行更改。当您拒绝这样的更改时，最好仍是向开发人员建议他们应该作些什么。

例如，您可能会说：“看起来您为此作了一些出色的工做，谢谢！可是，实际上咱们正打算移除这个系统，所以咱们如今不但愿对其进行任何新的修改。或许，您能够重构咱们的新 BarWidget 类吗？”

请注意，评审人员在拒绝当前的 CL 时要一并提出其余建议，并且还要表现地礼貌。这种礼貌很重要，由于做为开发人员，即便咱们不一样意，也要代表咱们彼此尊重。

若是出现多个 CL 是你不想进行的更改，则应考虑从新设计团队的开发流程或外部贡献者的已发布流程，以便在编写 CL 以前进行更多的沟通。最好在人们完成大量如今必须扔掉或完全重写的工做以前告诉他们“不”。

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

查找属于此 CL 的“主要”部分的文件。一般，逻辑更改数量最多的文件是 CL 的主要部分。首先看这些主要部分。这有助于为 CL 的全部较小部分提供上下文，并一般加快执行代码评审的速度。若是 CL 太大，您没法肯定哪些部分是主要的，请询问开发人员您应该首先看什么，或要求他们将 CL 分红多个 CL。

若是您发现 CL 的这一部分存在一些主要的设计问题，要当即回复开发人员，即便您如今没有时间评审 CL 的其他部分。实际上，复查 CL 的其他部分可能会浪费时间，由于若是设计问题足够严重，那么许多其余代码都会变得可有可无。

为何要当即回复开发人员有两个主要缘由：

• 开发人员一般会发出一个 CL，而后在等待审核时当即基于该 CL 进行新工做。若是您正在评审的 CL 中存在重大设计问题，那么他们也将不得不从新设计其之后的 CL。因此，最好赶在开发人员在有问题的设计上花费没必要要的时间以前告诉他们。
• 重大的设计变动比小的变动须要更长的时间。为了在截止日期以前完成工做，并在代码库中保留高质量的代码，开发人员须要尽快开始 CL 的全部重写工做。

第三步：按适当顺序检查其他的 CL

确认 CL 总体上没有大的设计问题后，请尝试找出逻辑顺序来浏览文件，同时还要确保不要错过对任何文件的评审。一般，在浏览了主要文件以后，按照代码评审工具向您展现它们的顺序浏览每一个文件是最简单的。有时在阅读主要代码以前先阅读测试也是有帮助的，由于这样您就能够知道更改应该作什么。

代码审查速度

为何代码审查应该很快？

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

当代码评审速度很慢时，会发生如下几件事：

• 团队的总体开发速度下降了。若是个体开发人员没法快速地对评审作出响应，多是由于他们有其余事情要作。可是，若是每一个 CL 都要等待一次又一次的评审，那么团队其他成员的新功能和错误修复就会被延迟了几天，几周或几个月。
• 开发人员开始抗议代码评审过程。若是评审人员仅每隔几天回复一次，但每次都要求对 CL 进行重大更改，那么这对于开发人员来讲多是很是使人沮丧且困难的。一般，这表示为对评审人员的“严格”程度的抱怨。若是评审人员可以快速提供反馈（确实能够改善代码运行情况的更改），抱怨就会消失，即便他们要求作出的修改是同样的。实际上，大多数对代码评审过程的抱怨均可以经过加快评审过程来解决。
• 代码的质量可能会受到影响。当评审速度缓慢时，开发人员的压力也会随之增长，由于他们不能提交不甚完美的 CL。缓慢的评审流程还会阻碍代码清理、重构和对现有 CL 作出进一步改进。

代码审查应该有多快？

若是你不是在集中精力完成手头的任务，那就应该在第一时间评审代码。

一个工做日是响应代码检查请求所需的最长时间（即次日早上的第一件事）。

遵循这些准则意味着典型的 CL 应在一天以内进行多轮评审（若是须要）。

速度与中断

有一种状况，我的速度的考虑赛过团队速度。若是您正在处理诸如编写代码之类的重点任务，请不要打断本身去进行代码检查。研究代表，开发人员在中断以后要花很长时间才能恢复到平稳的开发流程。所以，在编写代码时打断本身，实际上对团队来讲，要比让其余开发人员稍等一下子进行代码评审更为昂贵。

因此，对于这种状况，能够等到你手头工做能够停了再开始代码评审。能够是在完成手头的编码任务以后，午餐后，会议结束后，休息结束后等。

快速响应

咱们所说代码评审速度指的是响应时间，而不是 CL 经过整个评审流程并提交所花的时间。整个过程在理想状况下也应该是很快的，但单次评审请求的响应速度比整个过程的响应速度更重要。

即便有时须要很长时间才能完成整个评审过程，在整个评审过程当中得到评审人员的快速响应也能够极大地缓解开发人员对“缓慢”代码评审的不满。

若是您忙于在 CL 上进行全面评审时，仍然能够先发送一个快速响应，以使开发人员知道何时能够开始评审，或者建议让其余能够更快作出响应的评审人员来评审代码，或者提供一些初步反馈。

重要的是，评审人员应花费足够的时间进行评审，以确保此代码符合标准。但无论怎样，最好响应速度仍是要快一些。

跨时区代码评审

在处理跨时区代码评审时，请在他们仍在办公室时尝试与开发人员联系。若是他们已经回家了，那么最好能够确保他们在次日回到办公室时能够看到代码评审已经完成。

LGTM 带注释

为了加速代码评审速度，在某些状况下，即便审核人员也将未解决的评论留在了 CL 上，评审人员仍应该给予 LGTM/批准。在如下状况之一时完成此操做：

• 评审人员确信开发人员将会处理好评审人员给出的建议和意见。
• 其他的改动是次要的，不必定要求开发人员完成。

除非另有明确说明，不然评审人员应使用这些选项中的一个。

当开发人员和评审人员处于不一样时区时，带有注释的 LGTM 特别值得考虑，不然开发人员将等待一成天才得到“ LGTM，批准”。

大型的 CL

若是有人向您发送了一个很大的代码评审，您不肯定什么时候能够有时间对其进行评审，一般的响应应该是要求开发人员将 CL 拆分为多个相互构建的较小的 CL。 而没必要一次审查全部巨大的 CL。这样一般是合理的，而且对评审人员颇有帮助，即便须要开发人员作些额外的工做。

若是不能将一个 CL 分解为较小的 CL，而且您没有时间快速评审整个 CL，那么至少要对 CL 的整体设计写一些注释，而后将其发回给开发人员进行改进。做为评审人员，您的目标之一应该是是在不影响代码质量的状况下快速对开发人员作出响应，或者让他们可以快速采起进一步行动。

持续代码评审的改进

若是您遵循了这些准则，而且严格执行代码评审，则应该发现整个代码评审过程会随着时间的流逝而愈来愈快。开发人员知道为了保证代码质量须要作些什么，并从一开始就向你发送很是棒的 CL，这样评审所需的时间就会愈来愈少。评审人员学会如何快速作出响应，而且不会在评审过程当中增长没必要要的延迟。可是，不要在代码评审标准或质量上作出让步以提升速度 —— 从长远来看，这不会使任何事情更快地发生。

紧急状况

在某些紧急状况下，CL必须很是快速地经过 整个审核过程，而且质量准则将获得放宽。可是，请参阅什么是紧急状况？描述哪些状况实际上能够视为紧急状况，哪些不能够。

评审注释

摘要

• 礼貌。
• 说明您的理由。
• 在给出明确的指示与指出问题并让开发人员决定之间作出权衡。
• 鼓励开发人员简化代码或添加代码注释，而不只仅是让他们解释复杂性。

礼貌

一般，重要的是要礼貌和尊重，同时也要对要查看其代码的开发人员很是了解。一种方法是确保您的注释是针对代码，而是对开发人员。肃然没必要老是遵循这种作法，可是在说出可能会使人沮丧或引发争议的内容时，必定要使用它。例如：

很差的说法：“为何要在这个地方使用线程，这样作显然不会得到任何好处”

好的说法：“并发模型在这里增长了系统的复杂性，而我没有看到任何实际的性能优点。因为没有任何性能上的好处，所以最好将此代码为单线程而不是使用多个线程。”

解释理由

从上面的正确示例中能够看出，这样有助于开发人员理解为何要给出这些建议。您不必定老是须要在评论注释中包含此信息，可是有时适当的作法能够成为对您的意图的理解，所遵循的最佳实践或您的建议如何改善代码质量进行更多说明。

提供指导

一般，修复 CL 是开发人员的责任，而不是评审人员的责任。您无需执行解决方案的详细设计或为开发人员编写代码。

可是，这并不意味着评审人员不该该给予帮助。一般，您应该在指出问题和提供直接指导之间取得适当的平衡。指出问题并让开发人员作出决定一般能够帮助开发人员学习，并使代码评审更容易。这也能够带来更好的解决方案，由于开发人员比评审人员更接近代码。

可是，有时直接给出指令指令，建议甚至代码会更有帮助。代码评审的主要目的是得到最佳的 CL。第二个目的是提升开发人员的技能，以便他们以后的评审会愈来愈少。

接受注释

若是您要求开发人员解释一段您不理解的代码，他们一般会去重写代码。有时，在代码中添加注释也是一种适当的响应，只要它不仅是解释过于复杂的代码便可。

仅在代码检查工具中编写的说明对未来的代码阅读者没有帮助。只有少数状况能够接受这种作法，例如，你对评审的东西不太熟悉，而开发人员的解释倒是不少人所熟知的。

代码评审回推

有时，开发人员会回推代码评审。他们可能会不一样意您的建议，或者会抱怨您整体上过于严格。

谁是对的？

当开发人员不一样意您的建议时，请先花点时间考虑一下它们是否正确。一般，它们比您更接近代码，所以他们实际上可能对代码的某些方面有更好的了解。他们的论点有意义吗？从代码质量的角度来看，这有意义吗？若是是这样，请让他们知道他们是对的，而后问题就解决了。

可是，开发人员并不老是正确的。在这种状况下，评审人员应进一步解释为何他们认为本身的建议正确。良好的解释不只说明了对开发人员的理解，并且还说明了为什么要求他们进行更改其余信息。

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

有时，在真正提出建议以前，须要花不少时间去解释。请确保始终保持礼貌，并让开发人员知道您了解他们在说什么，您只是不一样意。

烦恼的开发人员

评审人员有时认为若是他们坚持要开发人员作出改动，会使开发人员感到沮丧。有时，开发人员确实会感到不高兴，但这一般是短暂的，以后他们会很是感谢您帮助他们提升了代码质量。若是您表现的颇有礼貌，那么开发人员根本不会感到不开心，这种担忧多是多余的。烦恼一般与评审人员的注释编写方式有关 ，而不是与评审人员对代码质量的坚持。

稍后解决

回退的一个常见缘由是开发人员想要完成任务（能够理解）。他们不想仅仅为了得到此 CL 而进行另外一轮评审。所以，他们说他们将在之后的 CL 中清理某些内容，所以评审人员如今应该 LGTM 此 CL。一些开发人员对此很是好，并会当即编写后续的 CL 来解决此问题。可是，经验代表，开发人员编写原始 CL 的时间越长，他们进行后续修复的可能性就越小。实际上，除非开发人员在提交当前的 CL 以后马上修复，不然在经过以后一般不会再去作这件事情。这不是由于开发人员不负责任，而是由于他们有不少工做要作，因此修复工做一般会被遗忘。所以，最好是坚持要求开发人员在代码进入代码库并“完成”以前当即修复其 CL 。让开发人员“稍后解决”是代码库退化的一种常见方法。

若是 CL 引入了新的复杂性，除非是紧急状况，不然必须在提交以前将其清除。若是 CL 暴露了一些问题，而且如今没法解决，那么开发人员应把错误记录下来，分配给本身，以避免丢失。他们还能够选择在引用已提交错误的代码中编写 TODO 注释。

抱怨评审过于严格

若是您之前对代码的评审不是那么严格，可是却忽然变得严格起来，那么一些开发人员将会大声抱怨。不过不要紧，提升代码评审的速度一般会使这些抱怨消失。

有时，这些抱怨可能须要通过数月的时间才会消失，可是最后开发人员会看到严格的代码评审的价值，由于他们会看到严格的代码评审帮助本身产出优秀代码。有时候，若是发生这种事情，声音最强烈的抗议者甚至会成为您最坚强的支持者。

解决冲突

若是您遵循上述全部内容，可是在代码评审过程当中仍然遇到没法解决的冲突，请再次参阅以上内容以获取有助于解决冲突的指导准则。

原文连接：How to do a code review