本文来源于我在InfoQ中文站原创的文章,原文地址是:http://www.infoq.com/cn/news/2013/11/code-review-why-when-how
对于很多公司来说,代码审查是开发人员日常工作中的重要环节。通过代码审查,可以及早发现项目中存在的问题、促进同事之间的沟通与交流,并且可以在讨论中迸发出智慧的火花。但要想成功实施代码审查却并不是一件轻松的事情,为什么要进行代码审查、何时做、如何做,这是摆在我们面前的3个重要问题。针对于这3个问题,开发者Lisa Tjapkes撰文谈到了自己的经验与教训。
在我最近的项目经历中,我们进行了广泛且正式的代码审查。这个过程极大地改进了项目的代码质量,降低了项目中新特性开发的等待时间,同时还促进了知识在整个 团队间的传播与共享。我还发现代码审查并不会延误项目开发的时间,反而会提升开发人员的生产力。
代码审查的好处
我们发现代码审查对于项目的各个阶段都会带来很多好处:
- 在项目起始阶段进行代码审查会帮助我们更好地使用已经建立起来的代码基,因为如果我们没有使用过某些现有代码,那么可以从当前的开发者中获得反馈信息。
- 在项目进行过程中,我们会时不时地向团队增加新的开发人员,代码审查可以极大地降低这些新加入的人员的熟悉时间。特别地,我们可以让新加入的开发人员很有信心地开发新特性,因为我们可以在合并前审查代码并且对于他们所编写的任何代码提供有价值的反馈信息。
- 对于我们这个分布式团队来说,代码审查更加具有实际意义。团队协同在构建协作环境上会带来很大的帮助作用,我们可以即时提出想法、然后讨论,再进行开发。虽然由于不在同一地点我们会失去一些东西,不过我们却可以在代码审查过程中通过深入的讨论来获得好处。
高效代码审查的技巧
代码审查的方式如果处理不当可能会导致项目延期。使用正确的工具与技术可以防止在审查上浪费大量的时间,提升代码的品质。
使用特性分支
这个实践的好处就不用多说了,不过它对代码审查提供了更加具体的好处。特性分支意味着你可以将需要审查的代码隔离到只与某个具体的特性相关。在代码已经准备好了审查时,特性分支还考虑到了快速的上下文切换。在当前的代码进行审查时,你可以切换到新的特性上来,如果需要对审查特性的反馈进行讨论时还可以快速切换回来。
将审查隔离为小的修改
相对于大的修改来说,小修改的审查时间会更少。在审查大的修改时,你不仅要看很多行代码,还要查看大量的依赖代码才能真正理解,结果就是花在审查代码上的时间与修改的代码量之间并不是呈线性关系。将待审查的代码隔离为小的修改可以降低审查者的精神负担并让审查过程更加顺畅。
使用专门为代码审查而设计的工具
这看起来似乎很简单,但却非常重要。一些重要的特性需要包含差异比较、能够逐行注释修改,并且在审查的代码发生改变时通知大家。我所在的团队使用GitHub来管理项目代码,并且使用GitHub的pull request特性来管理代码审查。
检查清单
可能没必要使用非常复杂或是过于结构化的东西,如果突然出现了某些问题,使用检查清单或许是个不错的主意。
有建设性的输入
类似于“多加点注释”这样的话显得太没意义了,帮助也不大。假如某个接口没有注释,但也许有专门的文档用来说明呢。如果发现了某些不合理的地方,那就要明确指出来,判断设计上是否能改进或是逻辑上是否存在着错误。
要把精力放在真正理解代码的行为上,确保当其他人需要维护它时也能够快速理解代码。
人人参与
即便是最有经验的架构师或是明星开发者也会犯错。因此,最好每个人都能参与到代码审查的过程中来。特别地,对于很多初级开发者或是新加入项目的开发者来说,这也是个很不错的学习机会。
要有审查流程
一开始,我们的项目并没有正规的审查流程。我们只是开启一个pull request,等待有人来审查,最后会有人合并修改。这种工作方式效率非常差,有时好几天都没人审查一个pull request,有时一个人给出反馈前其他人却合并了请求。此外,有些开发者审查的代码量要比其他人多不少。总而言之,没有流程导致我们的效率极低。
最后我们认识到了这一点,创建了一个正式的结构来指导该如何进行代码审查,这加速了审查的效率。一个审查过程至少应该涵盖如下几点:
- 如何将审查任务分派给不同的人
- 期待审查者给出反馈的最迟时间
- 如何标识某个审查已经完成
- 谁负责合并已经完成审查的代码
我在项目中是否应该进行代码审查?
我认为代码审查对于很多项目来说都是一件好事,不过它也并非通用的解决方案。代码审查适合于如下这些项目:
- 至少有5名开发人员
- 在向团队增加新的开发人员时
- 团队是分布式的
- 项目有各种不同的组件构成,这些组件是由不同团队开发的
- 当还处在为代码基设定约定以及最佳实践的阶段
- 团队中有些成员对于当前所使用的技术栈还不太熟悉时
相反,代码审查在如下的情形中并不会为项目带来更多的附加值:
- 处理的是维护代码而不是添加新的特性
- 团队很小且亲密无间
各位InfoQ读者们,你在工作中使用过代码审查么,你们的团队是如何做的呢,实践过程当中有哪些值得坚持的做法,又有哪些不太好的做法呢?欢迎大家一起讨论。