我们需要代码审查

说起代码审查,也就是CR(Code Review),大家都知道好处多多,但是真正踏踏实实实行下去又会碰到很多阻碍。下面我总结下这段时间在聚会玩工作中了解和实施过程中的一些想法和方法,分别按照轻重程度的顺序来说一说。

最初我对代码审查的要求是:

  1. 能够完美加入开发流的
  2. 尽量减少每个人审查的成本
  3. 真正提高代码质量

Phabricator

俗话说得好,不要重复造轮子。其实很多问题,我们碰到的,很多其他公司也同样会碰到,所以Facebook的牛人们自己开发了一套CR系统。这也是我最初推动内部CR时选择的工具。

优点

  1. 系统完善,从账号到和VCS(Version Control System)结合的很好,功能强大
  2. 完整的工作流,从review到audit,对应两种方式的审查方式(代码提交前和提交后)

缺点

  1. 过于笨重,文档繁琐,上手时并不是很流畅
  2. 界面很难看啊!!有木有!!

这个系统总体来讲是大而全的,什么东西应有竟有,但是给人用下来的感觉是有点古板(PHP大发好),按部就班,不出彩也没过失。

使用方法

  1. 同项目组内人员互相review
  2. 每天早上安排半小时左右review昨天提交的commit
  3. 每个人都会重复review同一个commit

在实际的使用过程中,经常会碰到一些review不过来的问题。6,7人的团队每天每个人都要处理开发分支上的大量commit。导致大家兴致底下,最后因为项目加班加点赶进度,直接导致大家把这事都忘了。

这种方式个人感觉还是比较适合大团队。

Gitlab的merge request

本身git工作流中就有merge request和pull request之分。在团队内部,在代码审查就可以利用merge request来进行

优点

  1. 较为轻量,不依赖外部系统
  2. 审查频率集中到一起,平时压力小

缺点

  1. 得安排大块时间共同review,或安排个别人review

这个方式是后来自己了解到的,可以针对之前失败的方式进行改造。实际也并没有实施过,但是可以想象的。这种方式当review一个merge request的时候,这个时候基本是一个功能完成的时间点,所以会涉及到比较多的代码文件和逻辑。所以这种方式平时轻松了,但是需要安排时间专门去review。

这种方式比较适合一些项目稳定,有新的小功能稳步开发时使用。

GitLab+Slack

最后是近期是使用的一种方式。现在的项目基本上都是2-3人一个小组进行。所以可以使用互相review的方式进行。gitlab+slack这种方式就最为轻量。slack只负责做通知,通知完了就完了。并不会有一个审查记录在。所以其实如果不看,都是不会有人发现。代码有问题就在gitlab中提issue。

优点

  1. 非常轻量

缺点

  1. 无法约束审查人

使用方法

  1. slack会推送每次提交的diff
  2. 也可以每天diff一下前一天的commit
  3. 互相review他人的代码
  4. 有问题提issue

这种方式我们团队在后期小组中也有使用。效果还是不错,主要是适用于人少的小组,互相review他人的代码非常的轻量,审查的时间也可以随时自己控制。

聊一聊通病

代码审查对于大部分程序员来讲的感受是,给自己多了开发之外的工作。所以先入为主的感受就是,所以如何调动和安排每个人的积极点和兴趣是非常重要的。所以可以加入一些激励机制,比如每天打卡,10天或者5天后会有神秘礼物,或者是一些荣誉上的奖励。这一块好像豆瓣的code系统做的还不错。

另外代码审查照理说也是开发的一个流程,但是国内大部分团队都基本忽视这个环节,所有的开发流程内都缺少这一环。所以要想代码审查有序进行,合理的开发流程也是非常重要。