设计
代码审核中最重要的事情就是考虑一下变更提交的整体设计。变更提交中各个部分的关联交互是否合理？这些变更是应该在代码基线中，还是应该提交到支持库中？这些变更是否能够与系统内的其他部分很好的整合？现在是不是加入这个功能的合适时机点？
变更提交是否实现了开发者的真正目标？开发者期望的对于代码的使用者是否有好的效果？这里的使用者包含终端用户（会受到变更提交的影响）和开发者（会在将来使用到这些代码）。
大部分情况下，我们都希望开发者在提交代码审核前能够事先做好充分的测试。但是作为审核者你还是要考虑边界情况，并发问题，尝试像用户一样思考，确保你审核的这些代码中不会产生任何bug。
当有必要时你需要验证变更提交的内容 - 当变更对用户交互产生影响，比如UI变更。有些时候单纯读代码是很难真正理解变更产生的影响。类似这样的变更，如果你直接从代码很难直接运行，可以要求开发者提供功能的演示。
另外一方面在审核代码，要特别重视类似并发等特别重要的概念，防止存在引发死锁或者竞争条件的写法。这类的问题即使运行代码也是比较难发现的，常常需要开发者和审核者仔细思考确保问题不会发生。（当可能存在竞争条件或者死锁时，尽量不要采用并发模型。这会导致代码审核或者理解都变得更加复杂。）
变更提交的内容是否必须这么复杂？每一级都要这样检查：单行代码是否过于复杂？方法函数是否过于复杂？类是否过于复杂？“过于复杂”通常意味着**”不能被读代码的人快速理解“。同样也意味着“当开发者尝试修改代码时更容易引入bug”。
一种常见的复杂化就是
过度设计**，将代码过度通用化，或者加入目前系统不需要的特性。审核者需要特别警惕这种过度设计。鼓励开发者针对当下需要解决的问题进行处理，而不是认为将来会需要解决的问题。将来的问题应该在出现的时候才需要解决，那时候你才能真正看清它实际的特征和需求。
针对变更内容，应该提供单元测试、集成测试或者端到端测试。大体上，应该在变更提交中包含测试代码，除非变更提交是因为处理紧急问题。
确保代码中的测试是正确、合理、有效的。测试代码不是为了测试本身，我们基本不会针对测试写测试代码，我们自己来保证测试代码是有效的。
当代码异常时测试是否真的会失败？如果基于测试进行代码变更，是否能够产生负的正向反馈？是不是每个测试方法内的断言都是简单有用的？测试代码在不同的测试方法中的分布是否合理？
切记测试代码也是代码，也同样需要维护。不要因为这些测试代码不在主代码中就允许它非常复杂。
开发者有没有设计良好的命名？良好的命名能够充分体现自己是什么或者做了什么，反之则会造成代码不具备可读性。
开发者有没有用良好表述的语言写了清楚的注释？是否所有的注释都是必须的？通常有用的注释是解释某些代码存在的原因，而不是某些代码在做什么。因为如果代码不能清楚的自描述，你就应该把代码处理的更简单一些。可能存在一些例外（例如：正则表达式，复杂算法常常需要注释来描述具体的工作内容），但是大部分的注释应该是说明代码不能包含的内容，比如做这个决定的原因。
在审核变更前先看一下注释也可能会有帮助。也许有一个待办项应该移除掉，一个注释的建议与实际的变更冲突等等。
注意：注释和类、模块、方法等的文档不同，文档是为了表达代码的目的，如何使用，使用时会产生何种效果。
在谷歌，我们提供了所有主要语言的《样式指南》，甚至包括大部分的低频语言。要确认变更提交是遵守了样式指南的。
如果你希望能够改进某些在样式指南中没有提到的样式点，在你的评论前加上”Nit:“的前缀，这样开发者就知道这是一个改进点而不是强制的。不应当因为个人的样式喜好而拒绝变更提交。
开发者不应当在主样式调整中混合其他提交。这样会使得变更提交中具体变化了什么内容，使得合并与回滚也更加复杂，还会引发其他问题。例如一个开发者希望重新格式化代码，那就应当将重新格式化的部分提交一次，其他的变更在这次之后另外提交一次。
#文档
如果一次变更影响到用户的构建、测试、相互引用、发布代码等，需要确认有没有更新关联的文档，包括 READMEs（项目自带文档），知识库页面，以及其他关联生成的文档。如果变更删除或者过期了代码，确认相关的文档是否被删除。如果文档没有更新或提交，要求要补齐。
#每行代码
仔细查看你分配到审核的每行代码。类似数据文件，生成代码，或者很大的数据结构之类的有时可以大概看一下，但是手写的类、方法或者代码段绝对不要大概看一下，假设不会有太多问题。有些代码就是比其他代码需要更加仔细的审核，这就是你作为一个审核者需要决定的，但是至少确保你理解了代码的逻辑。
如果发现看代码很困难并且导致审核的速度变慢，你应该让开发者明白问题所在并且等他们澄清整理之后再尝试审核。在谷歌我们只雇佣像你这样优秀的软件工程师。如果你都无法理解代码，那么其他的开发者也同样很难理解。当你要求开发者理清代码时，也是在帮助今后其他开发者能够理解代码。
如果你能理解代码，但是你觉得自己还不够资格来做某部分代码的审核，确保有另外一位有资格的审核者能够负责，例如安全、并发、可访问性、国际化等复杂问题。
#整体
将变更提交放到一个完整的上下文中来查看一般会更有帮助。代码审核工具一般都是展示变更附近的代码。有时候你是需要查看整个文件来确认这些变更是合理的。例如，你只看到添加了四行代码，但是当你查看整个文件会发现这四行是在一个五十行的方法中，而这个方法应该要拆分到更小的方法中。
同样将变更提交放到系统的上下文中作为一个整理来思考也是很有帮助的。这份变更是否会提高代码的健康程度还是是的系统更加复杂、更少的可测试之类的问题？**决不接受降低系统健康程度的变更提交。**大部分系统都是因为不断添加的小变更使得整体系统的复杂度不断提高，所以阻止新增提交中的复杂度提升是很重要的。
#闪光点
如果在审核中发现了代码中的闪光点，请务必告诉开发者，特别是他们用很棒的方式实现了你评审中的要求。审核者往往关注于问题，但是他们也应该欣赏和鼓励一些优秀的实践，这些行为在有些情况下更有价值，换言之就是要告诉开发者哪些做对了比告诉他们哪些做错了更重要一些。
当审核代码时，你应该确保：
代码应该是经过良好设计的。
功能都应该是对于用户有帮助的。（引用代码的开发者和终端用户）
任何UI修改都是合理并且展示良好。
所有的并发操作都要是安全的。
代码不应该过于复杂。
开发者专注于眼前的问题，而不要考虑实现将来可能要解决的问题。（不要过度设计）
代码中包含合理的单元测试。
测试的设计都是合理的。
所有的命名都很清晰合理。
注释清晰有效，大部分都是解释为什么，而不是做了什么。
代码有合理的文档（谷歌内部文档平台是g3doc1）。
代码符合样式指南。
确保你审核了每一行代码，联合了整体上下文，确保你通过的代码时提高了代码的健壮性，并且在开发者的闪光点上给了足够的鼓励和关注。
网友评论