谈目前项目组的代码提交制度

来源：http://www.cppblog.com/converse/archive/2010/07/08/119660.html

谈目前项目组的代码提交制度

(转载请注明出自 http://www.cppblog.com/converse 那谁)

一直以来想找机会谈谈目前这个项目组内采用的代码提交制度,今天整理一下.

分如下几个流程:
1) 在trac系统上建立ticket,写好这个任务的目的,并且accept这个ticket.
2) 修改代码,把相关的修改过的代码(一般还应包括相关的测试用例的代码,后面会加以说明)提交到review board上.
3) 在review board上填写如下几个必要的信息:
a)第1)步建立的ticket的号码(可以有好几个,也就是一次修改可以针对好几个任务),以明确这次提交针对的是哪个任务;
b)写下测试了哪几个测试用例,对这次的提交写一段描述.
c) 写下给哪些人做review.reviewer的角色分为两种,一种是负责人,另一种就是普通的组员.每一次提交,必须保证reviewer中有至少一个 负责人,并且需要所有的reviewer都通过了这次修改,才能向代码库提交代码.reviewer会针对提交的代码进行批注回复.
4)一般情况下,代码不会在第一次提交就能通过review,大多数情况会被打回修改,于是2)-4)三个步骤将循环进行下去直到代码通过review为止.
5) 提交了代码之后,1)中建立的ticket会被自动关闭,并且将在ticket的回复自动写上本次提交修改的文件以及svn revision号,这样以后再看起来就知道是哪次提交并且修改了哪些文件针对的又是哪个功能了.
6) 有一台服务器专门作为buildbot机器, 这台机器在每次提交了代码之后, 将自动清空原来的代码目录,更新最新的代码,重新编译,然后把里面的测试用例全部跑一遍.我们的项目使用的tcmalloc,会检查内存泄漏,所以在测试 用例测试失败,以及有内存泄漏的时候,buildbot都会失败.需要补充一点的是,除了每次提交代码会导致buildbot重新编译新的代码,在每晚的 一个固定时间,即使没有更新代码,也会做相同的动作.buildbot的存在,就是为了不断的清空编译文件重新编译再跑测试用例,以大量的测试消除随机性 保证正确性.

以上是整个代码提交机制的大体流程说明,下面谈里面的细节.

1)上面的第1)步中,建立trac的ticket时,需要指定一个milestone,一般我们对必须做的事情是每周以日期命名建立一个milestone,这样,你做的任务就会自然的变成每周可以去跟进的任务.
2) 第2)步中,写测试用例针对的是每个类或者每个头文件对外暴露的API接口,比如对外的API c使用内部的函数B,那么是没有办法对函数B编写测试用例的.我们的要求是, 任何的一个新增的API都需要写针对它进行测试的测试用例,不一定只有一个,因为需要考虑的情况可能很多,总而言之,尽可能的考虑齐全.假如本次修改修改 了内部函数B,那么依赖于函数B的API c它的测试用例也就需要再测试了.这些测试用的代码也会一并提交到代码库中,因为这样才能保证buildbot更新之后也按照最新的测试用例进行测试.
3) 代码提交到reviewboard之前,还需要过lint这一关,对基本的代码风格进行检查.我们使用的是google c++ code style.
4) reviewer中的负责人角色很重要, 起着看门人的作用,任何的一次修改提交,都必须至少经过一个负责人的review, 所以对他的要求就相对高些了,除了要完成自己的工作外,还需要认真review他人的代码,而要review他人的代码并且给出好的意见来,又要求他本人 除了编码能力外,还要在业务层面对别人的工作有大体的了解,不然没法review了.
5) 从前文可以看出,review实在是一个繁杂的工作,很有可能在review阶段被打回修改代码,就我的经验而言,提交review的人要将提交的任务尽 量的划分的细一些就来的很重要了.在提交review的时候,我一直坚持DOTDIW原则(Do One Thing,Do It Well).相反的例子,我们组有个同事,做了一个很大的功能,光是完成这个大的功能,就花费好几周的时间,提交review的时候代码量大,有个几千行 的,这样别人review起来也慢,而且一旦不通过需要修改,又是一个苦力活儿.这样一折腾,一个月时间过去了.如果当时能对整体的功能有个把握,懂得划 分模块层次,逐个提交,也许会好些.当然,大规模的代码提交有时并不能完全的避免,比如一个比较大的重构,牵一发而动全身的,我只是说如果可能,应尽量避 免大规模代码的提交,并且最重要的是:每次提交最好仅针对一个功能点.

以上是对流程从整体到细节的描述.现在谈谈我的看法.
先来谈优点:
1) 通过codereview制度,保证了项目组成员之间能够在代码层面上直接的进行交流.我想这一点是最重要的,没有之一.
如 果你是一个水平差一些的程序员,那么有比你牛叉的人帮你review,相当于是读书的时候有老师帮你阅卷修改作业,可以指正你的问题所在.我在被人 review的过程中就听到了别人对我很多的意见.而如果是一个水平较高的人,是不是对水平差的人进行codereview就是浪费他的时间了呢?我个人 认为,这个事情分怎么看.从团队的角度看,按照木桶理论,最短的短板往往决定了能达到的水平.总不能指望所有的事情都由老鸟完成,所以老人在帮新人 review代码的时候间接的帮助了新人的成长,同时作为项目组中资历比较深的人,也应该对项目多费一些时间进行把关.我觉得这一点无可厚非.同时,即使 是新人,通过阅读他人的代码并且交流,也可以学习到别人的思想.
codereview制度从上面的角度上保证了项目成员可以直接通过代码进行交 流,简单的说,谁写的代码质量如何,一到了review,一目了然.写的好的通过的快,写的不好被打回修改多了自己也会长记性,还可以多看别人的代码进行 学习.这样,在一定层面上可以使项目组成员的能力尽可能的接近.我能力长了一级,相应的也会拉动项目组中的人升级.

2) 测试用例.我之前提到的测试驱动开发,想法就来自于项目组中编写测试用例+buildbot执行测试用例的做法.如何证明一个API是确定无误的?我觉得 这个问题似乎非常难.除了一些可以通过数学上逻辑上证明的情况外,还需要考虑很多其他的随机情况.比如线程的切换是随机的,某个文件夹恰好存在是随机的, 等等.这些随机出现的问题,很多时候不能在某一次测试中显现出来.但是如果有了buildbot,不停的更新,编译,执行,总会有暴露问题的一天.另外, 之前我也提到过,脑中如果有了测试用例的概念存在,每写一个API时都会考虑到针对它的用例应该是怎样的,从另一个角度,也帮助你的设计--你需要考虑这 个API的输入,输出,异常情况都有哪些,如何测试到.测试用例的存在,保证项目尽量的做到了"可控制".

缺点:
1) 从上面的流程可以看出,走完一个代码提交的流程需要花费大量的精力/时间(有一些还是他人的精力/时间),所以,也许这个制度并不适合于那种时间压力比较大的项目.

2) 有几个地方很难坚持,比如codereview时,有些reviewer会走过场,也没怎么看代码就直接通过了,这样就会流于形式了.还有编写测试用例,也是一件很耗时间的事情.

3) 由于codereview制度的存在,给review中的负责人带去的压力很大,因为本身他有自己的工作,又要尽量对他人的业务/代码进行了解,就像那种 负载很大的服务器一样,有时候会不堪重负.这一点上,我的想法是,项目组内的其他成员,如果能积极主动一些,多花时间了解别人的代码,尽量分担 review的任务,或者试着成长为负责人级别(降低单点故障率:),也许是一个好办法.不过,这还得看人了.

整个代码提交流程上的软 件,目前我所知的都是使用的开源软件:SVN, google c++ code style lint,review board,buildbot,trac,测试用例的编写上使用的是google的测试用例框架gtest.不过,整合起这些软件搭建好这个流程就不是那 么容易了,用了一人/半年左右的时间,才让这个流程基本稳定下来.

总之,这套机制中,既有人为的干涉,也有机器层面上的干涉,也许并不是最完美的,但是尽了最大的力量去保证项目的正确性和可控性.

以正确的方式正确的制度做事,是将事情做好的一个关键.