谈一下我们是如何开展code review的
众所周知,代码审查是软件开发过程中十分重要的环节,楼主结合自己的实际工作经验,和大家分享一下在实际工作中代码审查是如何开展的。
笔者水平有限,若有错误和纰漏,还请大家指正。
代码审查的阻力
我想不通公司不同部门对代码审查这项工作的重视程度还是不一样的,对于代码审查的阻力总结了以下几点:
- 国内的整体环境,国内的公司,尤其是互联网公司,讲究速度致上,软件开发的迭代周期周期短,速度快,因为竞争太大,开发的产品要求快速上线,对代码审查不是很重视,先上线,出了问题再解决。
- 公司的规模,大公司重视流程,把代码审查作为软件开发中的重要一环,甚至计入考核,不管什么一旦成为制度,开展起来就相对容易了。小公司则不然,尤其是刚起步的,可能觉的代码审查没有必要。
- 和你的领导有关系,就和上面说的,代码审查如果没有形成制度,如果你的领导是技术出身,明白代码审查的重要性,那么会要求你去做。如果是来自别的领域,可能认识不到它的重要性,觉的代码审查是浪费时间(就和代码重构一个道理)。
- 个人原因,尤其是刚刚进入公司的员工,大学的软件工程课里面好像是没有介绍代码审查的,就是有,没有实际经验,也体会不到它的重要性,笔者刚入职时就是这么认为的。
代码审查的重要性
说了代码审查工作的开展遇到的阻力,下面说一下为什么代码审查是重要的。
- 代码审查是保证代码质量的重要手段。软件缺陷可能隐藏在各个地方,测试是发现缺陷的重要方法,但专业的测试人员更多的可能是黑盒测试,他们不去关注代码内部的逻辑,只去关注代码实现的功能,有人说测试代码中的逻辑需要开发人员进行单元测试,一方面,单元测试覆盖率基本上不可能达到100%,另一方面,毕竟是单元测试,测试场景简单,有些复杂的场景有可能会测不到。各种测试完成后,如果还有缺陷,那只能让客户充当我们的“终极测试”了。抱怨会接踵而来,客户满意度会越来越低。所以,我们要想出一切可以使用的方法来进一步提高代码质量的方法,还有代码审查么,测试发现不了的问题,通过代码审查也许你能够发现。
- 代码审查是熟悉软件架构,了解软件业务逻辑的好方法。学习代码是需要切入点的,一个上百万行代码的系统,从哪里开始着手,只能一个模块一个模块,一个组件一个组件的来熟悉,掌握。实现一个比较大的功能,你应该不会是唯一的开发人员,从系统架构师输出的系统设计,然后到各个团队中技术Lead输出的component级别的设计,到开始实现时,应该会把功能分为不同的模块有不同的开发人员协同实现。这是个学习的机会,不要只局限于自己这部分,为了了解这个大的功能,甚至和这个功能相关的其他已经实现的功能,你同样需要关注其他人的工作。有目的的看代码和漫无目的的浏览效果是不一样的,你已经对新功能有所了解,审查代码之前,你认为代码会怎么写,别人哪里和你想的不一样,旧功能和新功能是如何相互影响的等等,心里怀着问题,你的学习速度会更快,记得更加深刻。
- 代码审查是你提高自己的好方法。前提是team中有经验丰富的开发人员的存在。也就是大牛,不要错过让他看你代码的机会,不要害怕他会为你写的代码挑出一大堆问题,有人说你自己写的代码就像自己的孩子,见不得别人说半点不字,不要固执,要内心平静的,客观的去看待你所写的代码,发现并解决问题才能提高你自己。也不要错过去review大牛代码的机会,看看大牛写出来的代码是怎样的,你可以取其精华。
- 代码审查是需要功力的。网上有帖子说程序员的资深与否和工作年限没有必然联系,你是5年工作经验还是一个经验用了5年,这需要你去刻意练习,刚开始reveiew代码的时候你可能不习惯,也可能很痛苦,面对的一屏幕的代码不知如何下眼。但有一句话,如果你觉的内心很舒服,你就是在原地踏步。觉的痛苦说明你是在爬坡,刻意的去联系自己的大脑吧,今天你看一页代码可能用了一个小时,没有发现问题,但是坚持一个月甚至三个月之后,你看一眼就能够发现代码中的缺陷,恭喜你,你的功力加深了。
我们是如何开展代码审查的
好了。罗嗦了半天。下面开始说一下在楼主参与的项目中是如果开展code review的。
第一家公司,是一家国内的大公司,就不说名字了,我所在的部门开发的产品众多,换项目很频繁,我参与的有3,4个吧,开发流程不规范,部门老大没有对代码审查有硬性要求。但带我的老师,也是项目经理(但是主要做技术,所以也可以说是技术经理)是一个非常热衷于技术的人,应该说明白代码review的重要性,我们敏捷团队有4个开发,每次写完代码后,都会进行team review。把代码投到大屏幕上,然后老师带我们去review代码。印象深刻的一次是一个同事着急回家过年,草草把代码就提交走人了,被师傅挑出来很多问题。换了项目和项目经理之后,代码review就不了了之了。
第二家公司,是一个外企,有几十年的历史了,开发流程算是比较规范了,而且分工明确。在这家公司我们的大老板(也就是技术经理的上司)对代码review是有要求的,下面详细说明我们的代码审查是如何一步一步演进的。
- 第一阶段 team review + TFVC
先简单介绍下我们的版本控制工具:微软的TFVC,代码的branch是按如下图创建的,有一个main branch每个scrum team一个branch,出release之前把各个team的branch merge回main,最后出release branch,release branch上修复的bug也要最终回main。
开始的时候我们是没有peer review的,每两周开一次team review。一个主持人,负责预定会议室,操作visual studio查看最近两周提交的changeset,一个记录员,负责记录发现的问题,相关功能的开发人员负责讲解和解答疑问。最后记录员将review结果记录到wiki中并发送到整个开发部门。
- 第二阶段 自律TFVC + peer review + team review
记不太清是从哪个visual studio版本开始支持code review了,好像是VS2012。在提交之前每个开发人员需要将代码提交给至少一个人进行review,然后生成一个code review的work item。你需要将这个work item链接到你的changeset中才能check in代码,不然我们公司自定义的policy会发出警告。这些警告是可以被忽略的,然后也能强制提交。前面说过部分老大对code review是很重视的,如何才能检查peer review的结果呢?对,将这些code review的work item数据进行查询,将没有链接work item的changeset过滤出来,然后将结果显示。技术经理和老大一眼就能看到谁没有遵守这个流程。尽管这么做了,开始执行的时候还是有不少的人出现在查询结果中。
说一下自律的问题,公司添加这个查询review结果的措施是手段,只是在某种程度上保证了流程,但目的是什么?目的是需要收到review请求的成员认认真真的review代码,而不是随便的走一下流程就OK。如果你认识到review的重要性,你可能会用心一点吧。
我们的team review 会议依然在进行,和peer review的区别就是peer review只给一个人或者少数的人进行review,而team review 是在整个scrum team间进行。
- 第三阶段 GIT + peer review + team review
我们的公司虽然历史悠久,但对一些流程的工具和技术还是极力推崇的。大家都知道GIT是非常流行的版本控制工具,visual studio 2012也开始支持GIT,我们也一步一步的 将source code移到了TFS-GIT中。
和TFVC相比,GIT的branch是非常轻量级的,你可以很容易并且快速的创建一个branch。所以我们现在可以将branch进行细分了。TFVC和GIT的代码提交也不一样,TFVC是集中式的,最全的代码放在server上,你需要一个branch的code时要将其check out到本地。每次提交都是把代码从local一次性merge到server,如果出现conflicts,你需要在本地处理然后check in。GIT是分布式的,每个人clone的时候都会把所有分支download到本地,代码提交是通过pull request来进行的,也就是通过branch之间的merge来进行,这一点刚从TFVC转到GIT的时候很难理解。这样就得为每个人创建一个临时branch,注意这个branch在本地和server端同时存在,我们用这个branch开发自己的代码并用这个branch进行merge code。这里的pull request就相当于TFVC中的code review,TFVC你还可以偷懒忽略code review的work item,在这里就是强制性的了,没有pull request,别人不会approve你的代码,你根本就没有方法将你的代码merge到feature branch中。
还有team review会议也是照常进行的。