代码检视(Code Review)，源自Google的工程实践

来自Google的软件实践文档，标题是“How to do a code review"。文档包含6篇文章阐述有关代码检视的建议。本文对其内容进行解读，加深对当前公司牵引的committer文化的理解和实践。文章中的reviewers定位于公司的committers，CL(Change lists)对应于MR。
原文链接：https://google.github.io/eng-...

One. The Standard of Code Review

代码检视的原则。

1、代码检视的最高原则

代码检视的最高准则：”reviewers应当接受能够改进代码库健康度的CL，即使它们并不完美“。原因有两点，一是没有“完美”的代码，只有更好的代码；二是reviewer应当追求代码的持续的改进，包括可读性、可维护性等。

2、使用"Nit: "前缀的技巧

reviewer对提交comments不应当做限制，但是并非所有的comments都需要developer解决，此种类型的comments可以添加"Nit:"前缀，主要应用于两种场景：
1）提示此处可以更好，但是并不重要的comments；
2）帮助分享代码知识，从而更长期的改进代码库；

3、几条原则

1、技术事实和数据高于(overrule)观点和个人偏好；
2、代码风格规范是决对的权威：应当保持风格一致，如果没有规范，就接受原作者的；
3、除了风格，应当关注软件设计的原则，除非有正当的数据支撑，不应当违背设计原则；
4、没有规范时，可以保持一致，满足对代码健康度不劣化的目标。

4、解决冲突

解决冲突的首要是reviewer和deveoper对代码达成一致，在必要的时候组织会议而不是仅从注释来理解。如果仍然没有达成一致，要上升到管理或者技术专家决策，而不是放着CL不管。

感受：公司的编程规范应当作为编码的权威去遵守，但实际过程会听到很多关于编码风格和规范的抱怨，“为什么函数一定要小于50行？”、“为什么判空要遵循左变侧倾向于不变，右侧倾向于变化的规则？”，并且依旧我行我素的遵循个人喜好，这对于团队的编码和代码库的一致性破坏非常大。
规则应当严格遵守，当然规则也允许被质疑和挑战，因为规则也是在不断发展的，从1.0到3.2，规则不是一成不变的。希望大家拿出工程师解决问题的精神，对不合理的规则通过研究和数据推进改进，而不能只是抱怨和做规则的破坏者。在规则被重新制定之前，请严格遵守规则，业界的最佳实践要求函数NCNB就是应当<50行（职责单一、内聚、命名名副其实）。

Tow. What to Look For In a Code Review

 代码检视要关注的内容。

1、好的代码设计

修改的代码是否都有意义？分别归属什么版本库？是否和系统集成一致？从设计层面考虑代码是否符合好的标准

2、功能正确

确保功能正确，不仅指交付给终端用户，还要考虑代码未来的开发者，对UI的变更保证可视(demo)与美观，检视要考虑并发编程的安全可靠，考虑边界条件等，即使应该已经有测试用例来保证。

3、代码不应该比它所需的更复杂

复杂性要从代码行、函数、类等多个维度去考虑，导致代码复杂度增加常见有以下场景：
· 过度的代码设计（over-engineering）
· 解决当前的问题而不是解决未来的问题
代码的复杂度会直接影响代码的可读性和可维护性，也更容易在变更的时候引入问题。

4、单元测试

单元测试应当和提交的功能代码在同一个CL中提交；
单元测试本身不会被测试，编写测试用例的人应当保证正确，有意义；
单元测试本身也是代码库中的一部分，所以不要增加测试用例的复杂度。

5、命名

编码过程中有一半的时间在命名，确保命名清晰准确、有意义。

6、注释

注释应当解释why，而不是说明what。

7、代码文档

变更代码的同时，变更相关的文档。

8、代码风格

遵循代码风格，代码风格的重构和功能CL应该拆分提交，先重构风格，再提交功能CL。

对代码变更的每一行做检视，同时要结合代码的上下文（Context）而不只是工具展示出来的变更部分，这样才能判断出增加了几行，是否应该把一个超过50行的函数拆分成更小的函数。另外的建议除了聚焦错误，对好的代码应该赞赏developer，尤其是将commets修改后的好代码。

Three. Navigating a CL in Review

检视CL的方法。

1、通览整个CL

修改的内容是否有意义。如果没有意义，要有礼貌的拒绝：说明拒绝的原因，给出重构其他代码的建议。如果此类CL很多，就应当贴出通告，在大家投入之前声明”此项目即将废弃/进入维护周期，不接纳功能变更的CL“。

2、检视重点部分

找到CL中最重要的部分，通常是变更内容最多的文件。如果CL大到无法找到重点，则要求developer告知最重要的地方，或者要求其拆分成多个CL。
查看最重要的部分是否有设计问题，如果有应当立即将comments反馈给开发者，防止他们在原有错误流程上进行更多的工作，并且在deadline前重新完成交付。此时不需要检视CL中剩余部分的代码。

3、以合适的顺序检视剩余的部分

以合适的顺序，通常是按照文件的顺检视剩下的部分，确保不要遗漏任何一行需要监视的代码。

Four. Speed of Code Reviews

检视的速度。

当代码检视进行的很慢时，可能发生了：
1、整个团队的效率下降了；
2、开发人员开始抗议代码检视（day级的响应并打回CL，导致开发人员的抱怨）
3、代码质量下降。

1、代码检视应该多快

收到代码检视申请应该立刻响应，最长不超过一个工作日（通常是在第二个工作日的早晨）

2、如何处理打断

如果在集中精力处理一项任务或者在编码，就不要让code review打断自己，否则会需要小号更多的精力恢复原先的工作。建议在当前工作完成、开完会议、吃午饭回来、早晨刚到工位等时间进行检视。

3、快速的响应

即使CL很大，或者你没有时间进行代码检视，也尽量尽快的响应，并给初步给出一个整体的意见。

4、正向循环

长期坚持代码检视的原则，可以让开发者意识到好代码的要求，代码检视的速度也会越来越快，但是一定不能通过让步代码检视标准来提高检视速度。

Five. How to Write Code Review Comments

代码检视意见，应该写什么？

1、保持礼貌

评价代码，而不是评价人。不要用”why did you use threads here...“的语句。

2、解释为什么

通常情况下不需要，但是有时候需要解释为什么给出这样的评论。

3、给出指导

通常意义上，修改CL应该是开发者的责任，而不是reviewer。也不要你给出具体的解决代码。但是需要在指明问题和给出直接的指导之间权衡。
记住代码检视的目标，首先是尽可能得到更好的CL，其次的目标是帮助开发者提升技能，以便后续更少的代码检视。

4、接纳说明

不需要修改的检视意见，通常写在检视工具里，对以后的代码读者帮助并不大。除非那个检视意见是单独给reviewer解释，而大部分人明白的代码的含义。通常情况下，你需要找开发理解代码的意思时，应该使开发者重新简化他们的代码。

Six. Handling Pushback in Code Reviews

当开发者拒绝检视意见，或抱怨你太严格时。

1、谁是正确的？

开发者往往更理解所写的代码，所以应当停下来思考其抗争是否是正确的，如果正确可以接纳CL。
但开发者并不总是正确，此时应当要求其继续修改，直到能达成改进代码健康度的原则。为了避免几个回合中的抱怨，要保持礼貌，同时让开发者指导你在听他们说什么。

2、沮丧的开发者

可能会存在临时的沮丧，但事后会感谢你的帮助。如果作为reviewer礼貌的评论和快速的响应，实际上并不会导致开发者的沮丧。

3、稍后再清理

开发者会请求先合入，稍后再清理。原则应当是”立即清理“
1、对于影响较小的问题，可以再单独提交一个CL，并且是立即的。这不是说开发者不负责，而往往由于太多事情导致遗忘清理，"Later equals Never"。
2、对于引入复杂性的代码，必须清理后才允许合入

4、抱怨”严格“

如果你曾经松懈的审核，现在严格审核，可能会导致极大声的抱怨。加快你的审核速度可以让这种抱怨消失，这个过程有时候需要几个月，或者是在开发者看到你审核价值的时候。

5、解决冲突

并不是解决代码冲突，而是解决检视意见冲突。这个时候去遵循第一章代码检视的原则，它能帮助解决冲突。