Google代码检视指南-Code Review Developer Guide(译)

本文译自Google发布的代码检视指南，仅为个人理解，不代表任何官方，不准确的地方欢迎指正，原文路径：https://google.github.io/eng-practices/review/

Code Review Developer Guide

How to do a code review

Code 检视的标准

代码检视的主要目的是确保代码库上所有的代码保持健康，并且持续改进。所有的工具和流程都是为了这个目标设计的。

为了达到这个目的，很多地方是需要权衡的。

首先，开发者必须保持任务的进度。如果你从来不合入任何改善的代码，那代码库就不会有任何改善。当然，如果一个检视者很难对任何变更进行合入，那么开发者也就没用动力在将来进行任何改善。

另一方面，保持代码库的健康和质量不下降，是每个检视者的职责。这挺难的，因为往往整体质量的下降，是因为一笔笔小的降低质量的合入导致的，特别是团队在时间紧迫的时候，他们会认为走捷径才能打掉目的完成任务。

一个检视者也对检视的代码有责任和所有权。他们希望确保代码库是持续设计一致的、可以维护的、以及其他有关"在检视中应该关注什么"一节中提到的其他内容。

从而，我们得出我们希望在代码检视中遵循的规则是：

正常情况下，如果提交的代码比库上的代码有所改进，即使不是完美的，也应该合入。

这是整个代码检视指导的最高原则。

这当然有些例外条件：如果新增的特性设计的不是很好，或者这个代码不应该在此时出现，那也不应该合入。

关键在于，没有所谓完美的代码，检视者不应该在代码合入之前要求把每个细节打磨的很好。相反，相比不断提出改进意见，检视者更应该关注代码提交合入带来的进度不断推进。与其追求完美，不如追求持续的改进。只要一个提交对系统的可维护性，测试，可读性等方面有所改进，就应该毫不犹豫的合入他们，而不应该消耗太久的时间追求完美。

检视者，应该持续的去提示哪里做的更好，但如果是无关紧要的事情，可以加一些前缀让提交者忽略。

指导

代码检视活动始终是对开发人员学习新的框架、语言、软件设计原则和知识是有益的。分享知识也是不断提升代码质量的方法。如果你的想说的意见只是纯粹的教育，或者是本文档中提到的原则之外的东西，还是不要写到检视意见里了，或者加上Nit的前缀，让开发人员知道这不需要关注。

原则

• 始终要以科学事实和数据为依据，而不是观点和个人偏好。

• 样式相关的问题，以样式指南中的为权威。任何指南之外的东西，比如空格，以个人喜好为主，但是要保持一致。如果之前没有过类似的，那就以提交者的为准。

• 软件设计方面的问题，从来不只是样式或者个人喜好的事情。不能只看个人喜好，必须是基于基础的设计来评估。有时候看起来是合理的做法，如果提交者可以说明这些基于数据或者固定的设计原则的合理性，也应该通过提交。其他情况都应该基于标准的软件设计原则。

• 如果没有其他可用的规定，只要不恶化，应该和现有库里的设计保持一致。

解决冲突

首先应该基于本文达成共识。当达成一致变得很困难时，应该面对面沟通，而不是就在评审意见上聊。讨论的结果应该补充到意见上去。如果解决不了就应该找渠道上升问题，但是要围绕提交本身讨论。

代码评审应该关注什么

注意，关注以下几点的时候，应该时刻牢记代码检视的标准。

设计

提交的整体设计是否合理？提交的各个代码片段之间是否交互合理？提交的代码位置对不对？现在是添加这些功能的时候吗？

功能

代码是否代表开发人员想做的？可以给使用到这段代码的用户带来什么好处？有时候这段代码的用户就是他们自己。

大部分时候，代码应该是经过开发人员充分测试的。然而，检视者应该在审核的时候也按照用例去思考，通过看代码发现并发的问题，站在用户的角度上考虑，发现一些BUG。

如果需要的话，需要验证提交的代码，有些对于用户可感知的功能尤其重要，比如修改UI的代码。你可以要求开发人员提供demo，因为这种代码仅仅通过阅读是很难看出来实现的对不对。

另外一个更重要的是提交的代码里有没有并发的部分，这种BUG很难在运行时发现问题在哪。这也是要求如果有可能会导致资源竞争和死锁问题的时候，尽量不要用并发模型去编写代码。

复杂度

提交是不是比预期更复杂？需要从几个角度去评估：代码行是不是太多，方法、类是不是太复杂？太复杂意味着无法通过阅读很快的理解代码，而且调用和修改他们的时候，很可能引起BUG。

还有一种特殊的复杂度是过度设计。通常是开发者希望这代码比实际需要变得更通用，或者添加当时系统不需要的功能。检视者尤其要警惕这种情况，鼓励开发人员只解决当下需要解决的问题。而不是去解决那些猜想未来可能发生的问题。问题来的时候应该马上解决，你会看到问题真真实实的出现在面前的。

测试

提交应该经过了适当的单元测试或者端到端测试，有些时候也应该包含测试用例一起提交。除非是在解决紧急问题。

确保测试用例是正确、可靠、有用的。测试用例没法测试自己，我们也很少对测试用例写测试用例，我们应该保证测试用例是对的。

代码错误的时候测试会报错吗？每个测试用例都有正确而且有有效的断言吗？每个用例都是独立可靠的吗？

记得测试用例也是需要维护的，不要认为他们不会打到包里，就允许让用例变得太复杂。

命名

代码每个部分的命名是正确的吗？是不是足够长到可以描述清楚这个东西是个啥或者要干啥，如果命名太短可能没办法讲清楚。

注释

开发人员是不是用可以理解的英文(文字)写了清晰的注释？这些注释是必要的吗？通常来说，注释应该描述的是为什么代码要这么写而且写在这，而不是代码在做什么。如果代码没办法清晰的自注释描述自己在干嘛，那么这代码应该变得简洁点。但是比如算法或者正则表达式类的注释是例外，应该描述清楚在做什么。其他大部分情况应该描述一些代码本身无法承载的信息，比如讨论结论等信息。

查看这提交之前的一些注释也有所帮助，比如修改意见或者TODO之类的。

注意这里说的注释不是文件、类、方法头的文档注释，那些是用来描述代码模块的功能以及使用后起什么作用的。

风格

Google所有主力语言和大部分一般语言都有风格指导，确保开发人员已经遵守了风格规范。

如果你想提升一些风格规范里没提到的点，那最好在注释之前加上Nit，以说明这个不是强制的，只是更好的改进意见。不要因为个人的偏好阻塞提交的进展。

提交变更的人不应该修改主要风格，不然会让人很难理解代码本身的变更，会让merge和回退变得更复杂，还带来其他问题。比如说会有单独的代码格式化提交，然后另外又有真正的修改提交。