Code Review 新识

code review

我曾经写过一个有关Code Review的系列文章《代码走查究竟该关注什么》,在这个系列文章中我讲述了自己观察到的Code Review困惑并思考了做好Code Review应有的姿势、周边与工具,不过这些内容多是我个人的感性认识和实践小得。那么业界又是怎么看待和使用Code Review的呢?前段时间我读了一些有关Code Review的论文和资料,找到了不少有关Code Review的定量研究、分析和总结,刷新了我对Code Review的认知。

Code Review的定义

首先要分清什么是Code Review(代码审查,通常我们叫做代码走查),什么是Code inspection(代码检查),很多时候我们没有严格地区分两者,常常混为一谈。现代的Code Review有以下特点:

  • 非正式,不是一种会议形式的或者一种固定时间段的活动;
  • 基于工具,比如微软的CodeFlow,Google的Mondrian,Facebook的Phabricator以及普遍使用的Gerrit等;
  • 经常发生,由于伴随代码的提交,所以Code Review更加频繁。

而Code inspection的特点则是:

  • 逐行分组评审;
  • 会议上进行,可能会是一个单独的会议来进行检查,也可能是每日代码走查之后的一个固定时间。

显然Code inspection可能更接近我们平时常见的代码评审,一群人约定一个时间对代码进行检查,检查通常是逐行进行且基于一定组织规范;而在微软、Google以及Facebook,Code Review更加普遍一些,员工们依赖工具完成代码审查并在其上交换意见和建议。

微软的CodeFlow

尽管两者差异不小,不过在我看来,无论Code Review还是Code inspection都很有用,而且它们的生命力就是在于使用,至少在我的周边这两种方式实际上都在进行,我们既有每日站会后的固定时间Code inspection,也有基于Gerrit的Code Review。

可能是由于工作习惯的缘故,我周边的Code inspection让人感觉更有仪式感和参与度,一到这个时候所有人就被提醒现在是代码检查的时间,每个人自然而然地睁大眼睛、竖起耳朵来看和听每一段代码;而Code Review由于运用工具,因此不需要固定时间和地点,此外还附带跟踪和回溯。然而环顾我周边基于工具的Code Review,大家则大多将gerrit作为一个代码版本管理工具,代码检查则更多地交给流水线工具来操作,因此从gerrit上获得的建议和意见比较少。

Code Review的作用

如果我问这个问题,不少人的第一回答肯定是用来发现缺陷,微软的研究人员做过一项调查,他们统计了开发人员实施Code Review的动机,并进行了排名,正如我们想的一样,发现缺陷高居榜首,如下。

  1. 发现缺陷,高票当选的期望;
  2. 实现代码提升,代码提升是指在可读性、注释、一致性、无效代码删除等,但不包括代码的正确性或缺陷;
  3. 发现备用方案,采纳更好的实现方案;
  4. 知识传递,这种知识传递是双向的,不单单是被审查者,对审查者而言也是;
  5. 增强团队意识和透明,团队不仅必须知道代码所遵循的方向,而且不应允许任何人“秘密地”做出可能破坏代码或改变功能的更改;
  6. 达成共享代码所有权。
Code Review的动机排名

研究人员又记录了Code Review后的效果的统计,排名前三分别是:

  1. 代码提升
  2. 发现缺陷
  3. 知识传递

对比上面的期望发现不难发现,发起Code Review的动机与最终效果不太一致。尽管我们瞄准了发现故障的目标去执行Code Review,但实际上在Code Review过程中,我们做的最多是代码提升,比如符合编码规范与产品规约、提升可读性、补充或删除注释等。

Google很早就意识到Code Review在提升代码上的作用,最早在Google引入Code Review的人希望让开发者写出别人能够理解的代码。因此在Google,Code Review的首要作用是提升代码的可理解性和可维护性,此外还有检查实现风格与设计的一致性,确保足够的测试以及提升安全性。

对于发现故障来说,Code Review应当说是一种手段但不是起决定性作用的手段。比起发现故障,我刚看重Code Review的知识传递作用。这个其实也是第一个效果带来的,因为我们在纠正别人的代码同时,也在告诉别人为什么这样写不合适或者为什么那样写更合适,这其中包括规范、原则、实现模式和设计思想,对于新员工来说这样一种更加直接的学习途径,学习如何输出产品化、工业化的软件代码。

Code Review的要求

研究通过度量Code Review过程数据揭示了一些Code Review的属性,或者我们可以称之为实践要求。

评审频率与速度,在Google代码作者平均每周变动3次代码,80%的作者每周变动不到7次。同样,开发人员每周审查的变更的中位数是4次,80%的审查人员每周审查的变更少于10次;在速度方面,我们发现开发人员必须等待他们的更改的初始反馈,对于小的更改,平均时间不到一个小时,对于非常大的更改,平均时间大约为5个小时。整个审查过程的总体(所有代码大小)平均延迟不到4小时。

评审大小,在Google超过35%的修改只修改一个文件,大约90%的修改不到10个文件。超过10%的更改只修改一行代码,修改的行数中位数为24行。

评审者与评论数量,少于25%的变更有一个以上的审阅者,超过99%的变更最多有五个审阅者,审阅者中位数为1人;此外,每次更改的平均评论数随着时间的推移而增加更改的行数,对一个大约1250行的更改,每个更改可达到12.5条评论的峰值。

从这些数据中,研究者认为如果我们想要获得效果好的结果,必须要了解一些事实:

  • 评论简明易懂效果越好,快速地让代码作者找到问题才是好评论;
  • 代码评审是一个理解的过程,发现缺陷的前提是理解上下文,如果不理解代码就谈不上发现缺陷;
  • 如果参与评审的人具有背景知识,那么代码评审会更有效;
  • 作者可提供信息来协助评审者理解代码,事实上开发人员(作者与评审者)会通过各种各样的方式来完成代码理解这一过程,包括说明文档、邮件交流、当面讨论等。这其实是Code Review带来的社交属性
代码理解深度对达成Code Review效果的影响

对开发人员建议

总结上面的研究,研发人员需要关注的是:

  • Code Review不是免费的,Code Review带来诸如设计上更多的可维护性考虑、代码更加趋于一致性、更多知识分享,更多对变更发生的感知度以及额外的缺陷发现的好外外,也会引入时间的消耗(评审者阅读和理解的时间、作者定位反馈的时间、流程中等待的时间等),我们需要思考采用什么样的策略来应对如此多的变更;
  • 质量保证并不可靠,从期望和输出的对比看,依赖代码评审进行质量保证是不可靠的,可以作为一种辅助手段;
  • 理解是关键,团队应该致力于增加开发人员的对团队代码的理解程度这样才能提升Code Review的效果,同时代码作者也要肩负起让尽可能多的人来理解自己修订的义务(更加主动地发起评审),而不是遮遮掩掩害怕出错;
  • 超越缺陷,不要只看到缺陷,代码评审可以做的更多,特别是在知识传播上;
  • 沟通,Code Review是一项属于程序员的社交活动,既然是社交活动那就得多多沟通,沟通多了自然提升代码、发现故障与知识分享也就水到渠成了。

参考

  1. “Expectations, Outcomes, and Challenges”,Alberto Bacchelli,Christian Bird
  2. “Modern Code Review:A Case Study at Google”,Caitlin Sadowski,Emma Söderberg,Luke Church,Michal Sipko,Alberto Bacchelli
  3. “Code Reviews Do Not Find Bugs”,Jacek Czerwonka,Michaela Greiler,Jack Tilford
  4. 代码走查究竟要关注什么(一)
  5. 代码走查究竟要关注什么(二)
  6. 代码走查究竟要关注什么(三)
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 204,445评论 6 478
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 85,889评论 2 381
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 151,047评论 0 337
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 54,760评论 1 276
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 63,745评论 5 367
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 48,638评论 1 281
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 38,011评论 3 398
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 36,669评论 0 258
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 40,923评论 1 299
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 35,655评论 2 321
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 37,740评论 1 330
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 33,406评论 4 320
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 38,995评论 3 307
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 29,961评论 0 19
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 31,197评论 1 260
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 45,023评论 2 350
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 42,483评论 2 342

推荐阅读更多精彩内容

  • 代码静态分析 代码静态分析是指在不运行代码的情况下根据代码的静态信息,对代码的各个维度进行分析。 代码静态分析一般...
    MagicBowen阅读 3,530评论 0 1
  • 代码评审 代码评审也称代码复查,是指通过阅读代码来检查源代码与编码标准的符合性以及代码质量的活动。 代码评审应该与...
    梅哩哆阅读 1,882评论 0 0
  • Actual Fix Time 实际修改时间 Assigned To 被分配给 Closed in Version...
    社会主义顶梁鹿阅读 4,152评论 0 28
  • 其实在写这篇文章之前,对code review 已经有了基本的了解,不过没有深入下去,一方面懒,一方面也是想到领导...
    Cstars阅读 1,169评论 1 4
  • 16级考软件测试的时候,改为开卷考试。 当时准备了所有的英文ppt和这份中文材料加一本英汉词典,基本需要的东西都有...
    TulanCN阅读 3,007评论 0 3