effective-code-review

为什么每个团队都需要 Code Review?

不少开发团队和创业公司都在纠结是否要执行 code review,既希望改进代码质量,又担心带来的负担会拖慢项目进度。实时上,在软件开发中质量和效率往往并不是二选其一的关系。能产出高质量代码的团队通常效率也非常高。

在我作为工程师的职业经历中,自动化测试和 code review 可说是能同时提高代码质量和开发效率的两个最有效的手段。所谓 code review,和学术界的 peer review 类似。Peer review 是由同事或同行对一位作者的作品进行查阅并提出建议和问题,只有当所有提出的问题都得到满意的答案后,作品才能发表。对于 code review 来说,作品就是代码,发表就是把代码 commit 到官方代码库。

在 Code Complete 这本书中讲述了两个很有说服力的案例。在一项对同一个团队开发的很多个程序进行对比的研究中,没有经过 review 的程序平均每 100 行有 4.5 个错误,而经过 review 的程序平均每 100 行只有 0.82 个错误,也就是说 80% 的错误在 review 中被修正了。AT & T 的一个 200 多人的部门在开始执行 code review 后,开发效率提高了 14%,而错误减少了 90% 左右。

除了减少缺陷,避免在诊断错误上浪费时间,review 的过程还可以通过相互的督促保证代码有好的可读性、文档、风格,并同时检查测试覆盖率等开发过程中的规范,从而提高团队的协作效率。对于所有复杂的事情来说,总是越早发现问题,解决问题的成本越低。

对于经验不足或者刚开始一份新工作的人来说,通过 code review 可以得到更资深的人帮助,更快熟悉现有的规范和架构,在新的环境和团队中快速提升。

对于资深的工程师来说,让其他同事 review 代码,有利于在团队中传播经验、知识和好的实践。身边的同事水平提高会让自己的工作也更高效。并且谁都有需要休假的时候,无论是公司还是个人都不希望有太多工作因此而停滞,如果有平时就熟悉自己工作的同事,这个问题就很好解决。

像很多其他事情一样,code review 最难的就是迈出第一步。一旦开始,花在 review 过程的每一分钟都会很快被成倍地赚回来。如果你不在一个可以一下改变团队流程的位置上,那么至少可以和认同这件事的少数同事先开始实践,当价值开始体现的时候,相信其他人会乐于效仿。

怎么才能让 code review 有效发挥它的作用。

**使用方便的工具。**要在团队中成功推行 code review,有好的工具是很重要的。谁都不愿意在工作中加入繁琐的过程,所以如果要让人接受一个新的步骤,最好让它有尽可能好的体验。在我用过的开源 code review 工具中,Phabricator 是最完善的。Review Board 应该也是不错的选择,只是我很久没实际用了,不知道最新状态。GitHub 在支持 side-by-side diff 后,它的 pull request 也提供了很好的 code review 体验,唯一不足的是无法从机制上强制所有 commit 都经过 pull request 的过程。

**把简单的检查自动化。**有很多检查是可以自动化的,比如一些风格规范(缩进、空行、行尾空格、命名等),这类问题应该尽可能写脚本检查。一方面可以让负责 review 的工程师把更多注意力放在更高层面的问题上;另一方面,从接受 review 的人心理上说,由程序提出这些细节问题比让另一个人来挑刺要更容易接受些。当然,这并不是说人工 review 的时候应该忽略细节问题。

**控制每次 review 的代码量。**每次 review 包含 200 行左右代码是比较理想的,最多不要超过 400 行。因为如果代码太多,review 的人容易因为注意力分散而忽略一些问题,另外也可能让时间拖得过长。因此,开发的时候需要把大的改动分解成多个小的步骤,每完成一个步骤就提交一次 review。

**使用异步的工作流。**这和上一点是相关的,当你需要把一个改动分为相互依赖的多步时,不应该因为等待 review 而 block 住自己的工作。在当前分支等待 review 时,可以从这个点开一个新的分支继续开发,之后再把 review 完的分支 merge 进来。

**作者应该提供清晰的 commit note。**Code review 的重要作用之一是同事间的交流,每个 commit 的 commit note 很重要也很影响 review 的效率,应该包含这个改动的目的,以及实现方式的概述。如果使用的 review 系统支持对 review 本身的描述(如 GitHub 的 pull request),那么应该写清楚作者希望 reviewer 重点关注的问题。

**在合并到主干之前进行 review。**有的人主张小团队应该做事后的 review,因为这样效率更高、更「轻量」。事实上,这样并不会减少工作量,并且如果工作流安排合理,事前 review 并不会导致效率降低。从心理上说,代码并合并到主干之后往往就意味着「完事了」,在时间比较紧张的时候很难坚持 review 所有代码。所以应该把 code review 作为代码合并到主干前必过的一道关口。

**所有人的代码都要经过 review。**Code review 并不是资深工程师对初级工程师做的事情,而应该全员平等参与,每个人都会有所收获。关于这方面在上一篇「为什么每个团队都需要 Code Review」里也有提及。

**关注设计方面的问题和客观的规范,避免在主观意见上争执。**Code review 的讨论应该专注于设计层面的问题以及在团队中有明确共识的规范(代码风格,测试覆盖等),而要避免在一些主观意见的分歧上浪费时间。这也意味着应该在一些本身不重要,但很影响一致性的细节上尽早达成有共识的规范,如缩进方式,括号的位置等等,避免在 review 中去争论这些问题。

**从正面看待在 review 中发现的问题。**当发现一个错误时,并不应该看作是一个人犯了错被另一个人发现了,而是两个人配合改进了代码、避免了错误。

**Review 所有的代码,包括很简单的改动。**每一个 commit,无论多小都应该经过 review。一方面,很难界定什么算简单的改动,如果一行的 commit 不用 review,那两行是不是差不多同样简单,三行呢?另一方面,我已经不记得自己曾有多少次觉得「这个改动太简单,不用跑测试就可以提交了」,然后很快因为测试通不过而很尴尬地被别人 rollback。所以无论多小的改动都应该有测试、都应该通过 review。

**循序渐进。**如果现在完全没有做 code review,那么到整个团队都能严格有效地进行 code review 是一个循序渐进的过程。如果你所处的角色无法很快改变团队的工作方式,那么可以从把自己的代码发给别人 review 开始,实际可见的价值是最有说服力的。

code review wtfsMeasurement of Code Quality