code review平台Rietveld应用指南

 

【摘要】本文以step by step的方式对Rietveld code review平台如何在实际的测试项目中应用进行了介绍，并对code review流程的两种不同实施方式，pre-commit review和post-commit review进行了说明。
【关键词】 Rietveld、code review、code review流程、pre-commit review
1 引言
1.1 背景
目前公司在项目的开发和测试过程中开展code review取得了一定的成效，但也存在明显的不足：
 ·code review缺乏统一规范的流程性要求，实施起来较为随意，没有适合的方式对code review的环节进行分析评估和职责界定，导致code review的实施效果并不明显
·code review记录的整理需要手工完成，不能统一管理，review过程中发现的问题清单由个人保管，缺少有效的公开途径，不能达到分享他人经验、提升自身技能等预期的目标
·review过程中对于已发现问题修改情况的追踪主要使用手工的方式完成，执行起来过程繁琐，容易产生疏漏，导致开销较大，工作效率也会有所降低
1.2 综述
Rietveld正是一款解决了上述问题的code review工具平台。Rietveld是由python脚本语言的创始人Guido van Rossum发起的一个开源项目，工具前身为Guido van Rossum本人为google公司研制的Mondrian。Mondrian工具采用google公司私有的数据存储技术BigTable和商业化的版本控制工具Perforce设计实现。
为了能够促使Mondrian工具平台所包含的CR实施流程能够在更大的范围中采用，扩大google公司的影响力， Guido van Rossum基于Google公司的另一项技术Google App Engine（GAE）虚拟主机平台创建了Rietveld开源项目。Rietveld使用python的web开发框架Django设计实现，集成了包括开放源码的版本控制工具SVN在内的多款版本控制工具。Rietveld开源项目的所在网址为http://code.google.com/p/rietveld/。
1.3 术语解释
1.3.1 pre-commit review
在代码check in(ci)进开发库之前进行代码的review流程，通常这是一种更为严格的方式，因为在提交代码至代码库之前进行code reveiw将使得开发库中的代码具有较少的问题。
pre-commit review方式的实施流程为：在本地的copy中通过版本控制工具制作diff，然后基于该diff在code review平台上提交review request，在request中指定diff所处的开发库路径。reviewer接收到request之后，可以直接review diff文件，或者进行side by side方式的review（需要工具支持），也可以将diff及其base url下载到本地，选择合适的IDE工具进行review。
一般认为，rietveld是一款基于pre-commit review方式的代码评审平台，也就是说只有经过review的代码才可以检入到开发库中。
1.3.2 post-commit review
当代码checkin后进行reivew，即对开发库中已提交的源代码版本进行review。
rietveld提供了有限的post-commit review方式的支持。upload.py脚本提供了—rev参数，当对该参数指定—rev=N:M参数形式时，表示对开发库中的N和M版本之间的diff创建review issue。
就目前code review的实施现状而言，post-commit review还需要在多个项目的测试过程中采用。
2 quick start
2.1 overview
本章节主要结合目前百度技术部门开发过程中存在的两种不同的项目开发方式，对如何在测试项目中使用Rietveld开展code review应用流程进行了说明。用语“全新开发项目”表示对开发库还尚未执行提交操作，开发库中内容为空的软件开发项目类型。用语“升级开发项目”意指开发库中已经存在提交的版本，主要进行升级的项目开发类型。
2.2 全新开发项目
对于一个全新的项目开发过程，我们首先需要设置开发库中的项目路径。
例如我们要开发的项目名称为testproj，采用主干开发的方式，开发库路径信息如下图所示：
 

 

此时开发库中各预设路径下所包含的内容皆为空。
RD在自己专属的开发机上编写代码，在向开发库提交代码前（check in），需要首先进行code review。
rd从库中检出checkout trunk上的代码：
[[e-mail] opencode]$ pwd
/home/zhouwei/svnroot/opencode
[[e-mail] opencode]$ svn co https://svn.baidu.com/opencode/trunk/zhouwei/testproj/trunk testproj
Checked out revision 1103.
将编写的代码拷贝到新检出的路径当中，同时也将upload.py拷贝至该路径下：
[[e-mail] testproj]$ pwd
/home/zhouwei/svnroot/opencode/testproj
[[e-mail] testproj]$ ls -l
total 100
-rw-rw-r-- 1 zhouwei zhouwei 1075 Feb 21 11:33 include.h
-rw-rw-r-- 1 zhouwei zhouwei 294 Feb 21 11:33 StdAfx.cpp
-rw-rw-r-- 1 zhouwei zhouwei 769 Feb 21 11:33 StdAfx.h
-rw-rw-r-- 1 zhouwei zhouwei 2373 Feb 21 11:33 TestCon.cpp
-rwxr-xr-x 1 zhouwei zhouwei 81591 Feb 21 11:33 upload.py
注意，upload.py应具有“可执行”权限。
RD在开发机上输入命令查看此次更新的状态
svn st
例如：
[[e-mail] testproj]$ svn st
? StdAfx.h
? TestCon.cpp
? include.h
? StdAfx.cpp
? upload.py
upload.py是我们使用添加issues到rietveld平台上的脚本文件，不希望被添加到开发库，因此需要将其加入到svn的ignore列表当中。
在将其加入到svn忽略列表之前，首先必须要指定svn可以使用的编辑器，这里我们使用vim作为svn可以使用的编辑器：
export SVN_EDITOR=vim

然后，通过svn提供的属性编辑功能为目录添加忽略的文件：
svn pe svn.ignore .
在vim打开的文件中输入要忽略的文件列表，在这里我们输入upload.py，然后保存并退出vim。
Set new value for property 'svn:ignore' on '.'

使用svn add命令添加此次修订增加的源文件，svn add命令只更新本地拷贝，不会访问开发库中的内容。
[[e-mail] testproj]$ svn add *.cpp
A StdAfx.cpp
A TestCon.cpp
[[e-mail] testproj]$ svn add *.h
A include.h
A StdAfx.h
注意，不要使用svn add命令添加upload.py脚本文件。当以上操作都完成后，通过命令svn st查看当前目录的更新状态：
svn st
svn会输出：
[[e-mail] testproj]$ svn st
M .
A StdAfx.h
A TestCon.cpp
A include.h
A StdAfx.cpp
可以看到，upload.py文件已经被svn忽略，不再显示其版本控制的状态。
如果希望撤销先前所执行的svn操作，应执行svn revert -R命令，“-R”参数表示操作要递归到所有子目录。
svn revert -R .
在提交cr之前，我们可以执行svn diff命令，再次对此次更改的全局内容进行确认：
svn diff
然后在shell命令行中输入：
[[e-mail] testproj]$ ./upload.py -e duquanxi -r zhouwei -d "first import2" --send_mail
命令行中，“-r”参数指定参与review人员的名称，“-d”参数指定此次提请review代码的描述内容，
命令执行过程中，upload.py会询问发起issue的主题（subject）以及在rietveld中的用户名和密码等信息：
Upload server: http://baidu.com:8010 (change with -s/--server)
New issue subject: first import
Loaded authentication cookies from /home/zhouwei/.codereview_upload_cookies
Issue created. URL: http://baidu.com:8010/6
Uploading base file for StdAfx.h
Uploading base file for StdAfx.cpp
Uploading base file for TestCon.cpp
Uploading base file for include.h
Uploading base file for .
rietveld会向reviewer发送一封email，包括提请review代码的url地址、待review的文件清单等内容，如下所示：
Reviewers: zhouwei,

Description:
first import2

Please review this at http://

Affected files:
M .
A include.h
A StdAfx.cpp
A StdAfx.h
A TestCon.cpp
在命令行中加入--noisy选项，upload.py将输出执行过程中的trace信息。
reviewer接收到邮件后，点击其中的url链接将打开此次提请review的包含指定issue的页面。reviewer也可以登录到rietveld平台，查看需要进行reivew的issue。
rietveld提供了两种不同的源文件展现方式：
 ·unified diffs：以unified diff的格式显示待review的源文件与开发库中对应文件之间的diff
 ·side by side diffs：以side by side的方式显示待review的源文件与开发库中对应文件之间的diff，左侧显示开发库中的对应文件，右侧显示待review文件的内容
 

reviewer可随时在review的过程中双击代码中的任一行，对该行添加comment，如下图所示：
 

图中左上角的“Draft”字样表明这个comment属于草稿，还需等待reviewer确认后方能够发布。
新编辑的内容属于草稿（draft），必须要执行“Publish+Mail Comments”操作后，其他用户才能够看到新添加的comment。
返回到issues列表页面后，此时页面将以加重红色文本的方式提醒reviewer执行“Publish+Mail Comments”操作：
 

点击“Publish+Mail Comments”链接后，rietveld显示：
 

点击“publish all my drafts”按钮后，一个comment便添加完成了。
此代码的“owner”（这里是duquanxi）将会收到一封提醒邮件，内容如下：
错误处理还需加强

File TestCon.cpp (right):

TestCon.cpp:110: FILE *fp_ = fopen("123.txt", "r");
没有判断fp返回值
程序结束没有关闭打开的文件句柄

“owner”（reviewee）接收到邮件或者直接登录到rietveld平台，可以看到reviewer添加的comment，reviewee可以reply这个comment。若reviewee接受reviewer的意见，可以快捷回复“Done”，表示接受这个comment。否则，reviewee附上自己的看法通过rietveld平台与reviewer进行沟通，这个沟通反馈过程可能会持续多个回合。
reviewee对reviewer提出的问题fix之后，可以使用“./upload.py --issue=no”命令提交新版本review，其中no表示提交patch所针对的issue ID，例如：
[zhouwei@testproj]$ ./upload.py --issue=8
Upload server: (change with -s/--server)
Message describing this patch set: fix the file pointer check
Loaded authentication cookies from /home/zhouwei/.codereview_upload_cookies
Issue updated. URL:
rietveld能够对基于同一issue的不同版本的patch进行管理，如下图所示：

若reviewer对reviewee提交的patch没有其他意见，则可以使用inline comment或者“send message”的方式发布通过review的意见。表示通过review的用语可以是：LGTM(look good to me)、同意等。
当issue被approve后，reviewee应关闭该issue（只有代码的owner才可以执行关闭issue的操作），然后再提交经过review的代码到开发库中。
owner可以在issues列表页面上直接点击指定issue左边的关闭图标，如下图所示：
 

一次完整的code review流程便结束了。
2.3 升级开发项目
对于升级开发项目，除了文件间的diff之外，我们可能还需要对升级之前的base版本包含的所有文件进行review。
如前所述，rietveld使用diff技术生成待review的文件清单，如果开发库中存在已受控的软件源代码，rietveld在根据本地的工作拷贝和开发库中指定的源代码版本创建diff时，不会生成完整的源代码清单，所以似乎没有办法对开发库中已有的revision进行review。
不过，我们可以采取“变通”的方法来绕开这个问题。
reviewee应首先从开发库中检出已有的版本：
[zhouwei@testproj]$ svn update
At revision 1105.
然后执行svn del命令，删除本地拷贝中的所有文件。svn del PATH命令只会影响本地的workcopy，不会对开发库造成任何影响。
[zhouwei@ testproj]$ svn del *
D include.h
D StdAfx.cpp
D StdAfx.h
D TestCon.cpp
svn: Use --force to override this restriction
svn: 'upload.py' is not under version control
将本地copy清空后，执行upload.py脚本生成完整的diff文件清单
./upload.py -e zhouwei -r duquanxi -d "review entire" –send_mail
rietveld将会显示所有被删除的源文件：
 

最后我们再对本次workcopy执行revert命令取消先前进行的所有操作。
svn revert --recursive .
如果我们采用的版本控制策略首先完成的是创建库结构的操作，那么可以很简单的完成对指定版本源代码的review issue的发起。例如：
 

1101只是完成了开发库结构的创建，并没有import或者commit任何源文件
revision 1105 完成了首次源代码的提交
那么我们可以使用upload.py脚本完成post-commit review的功能：
[[e-mail] testproj]$ ./upload.py --rev=1101:1105
参数--rev=m:n的含义为创建基于revision m和revision n之间diff的review issue
Upload server: http://baidu.com:8010 (change with -s/--server)
New issue subject: po
Loaded authentication cookies from /home/zhouwei/.codereview_upload_cookies
Issue created. URL: http://baidu.com:8010/10
Uploading base file for StdAfx.h
Uploading base file for StdAfx.cpp
Uploading base file for TestCon.cpp
Uploading base file for .
Uploading base file for include.h
rietveld将显示：
 

之后的code review执行流程与“全新开发项目”一节中阐述的内容基本相同，这里不再加以赘述。
3 指导性原则
3.1 issues
在rietveld中，每次发起的一轮代码评审请求称为一个issue。每一个issue具有subject、Description等描述性信息，在使用upload.py脚本创建issue时，需要正确的指定issue的subject和Description。
3.1.1 主题（subject）
一个issue的subject描述内容应简洁清晰，内容一目了然，便于reviewer能够直接从issue的subject描述中获得此次reivew的重点，提高CR的效率。
一个好的subject描述风格是：
 ·“添加了日志功能”
 ·“修改了XX功能的BUG”
 ·“对XXXX函数进行了优化”

不好的subject描述风格为：
 ·issues1：？？？
 ·修改：描述内容不确切
3.1.2 描述（Description）
Description在内容上应对subject做进一步的阐述。如果可以，Description应包含此次code review要求的详细说明。
例如：如果subject为bugfix，则应在Description中指明修复bug的ID。
另外最好指明此次提请review issue前RD所做的测试类型，例如：
BUG=chromium-os:6268
TEST=manually test.
3.2 评论（comment）
rietveld提供了两种不同的comment方式，一种为inline comment，也可以通过reply message的方式对整个issue进行comment。
在添加comment时，我们希望可以使用一些固定用语来表达“接受comment”的含义，本文提出一些推荐的单词供参考：
 

3.3 提交（commit）
当 rd的代码通过了review准备提交代码时，应在commit日志中添加提交版本进行code review的issue id以及reviewer的用户名称，例如：
commit日志：
issue=14
reviewer=zhouwei
4 FAQ
Q：rietveld能够提供基于project的issues管理功能么？
A：rietveld关于issues提供的管理功能很有限， rietveld目前仅能够按照“star”、“owner”、“closed/open”等字段搜索符合要求的issues，并不能对issues进行其他的分类。

Q：执行upload.py脚本时，为什么会输出No output from ['svn', 'diff']
A：rietveld采用基于diff的方式创建需要进行review的文件清单，若本地工作拷贝与开发库对应revision包含的内容相同，upload.py脚本在执行的过程中调用svn diff创建diff文件，若diff文件为空，则会输出“No output from ['svn', 'diff']”。

Q：upload.py脚本都提供了哪些参数，其功能都是什么？
A：upload.py提供了较丰富的选项，详细内容可以参考官方网站提供的说明：http://code.google.com/p/rietveld/wiki/UploadPyUsage
本文认为需要重点掌握的选项包括：
Logging options
-q, --quiet
Print errors only.
-v, --verbose
Print info level logs (default).
--noisy（用来调试）
Print all logs.

Review server options
-s SERVER, --server=SERVER
The server to upload to. The format is host:port. Defaults to '%default'.
-e EMAIL, --email=EMAIL
The username to use. Will prompt if omitted.

Issue options
-d DESCRIPTION, --description=DESCRIPTION
Optional description when creating an issue.

patch
-i ISSUE, --issue=ISSUE
Issue number to which to add. Defaults to new issue

--rev=REV
Base revision/branch/tree to diff against. Use rev1:rev2 range to review already committed changeset.（提供了简单的post-commit方式支持

Q：RD应多长时间应提交一次？
A：根据一些专业机构的统计数据表明，code review的最佳实践是：
 每次review的时间1.5~2h
 每小时review的速率在300~500行
为了提高code review的效率和产出，我们建议RD每次check in的代码行数在500行左右，这样reviewer可以控制在1个小时候内完成提交代码的code review。

（baiduqa，[e-mail]）

 【本文转自百度测试技术空间】http://hi.baidu.com/baiduqa/blog/item/ae814352c598fd491038c2b5.html

【关注百度技术沙龙】