让 Code Review成为一种习惯

1. 开篇

5月份的时候忽然接到 code.oa.com【腾讯内部的一个代码管理平台】 的 summer 的通知， 说广点通的codereview 参与度在公司各部门中表现出色，而咱们小组（广点通广告定向小组）的 codereview 综合表如今全公司的小组中排名第一。这让我有点意外，有点无意插柳的感受。 summer 但愿我能写写咱们小组作 code review 的一些心得体验， 回想我学习使用 codereview 的经历， 我最早想到的一句话是我几年前看到的关于 codereview 的一句评述：

The biggest thing that makes Google’s code so good is simple: code review.That’s not specific to Google – it’s widely recognized as a good idea, and a lot of people do it. But I’ve never seen another large company where it was such a universal. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.

这是一段让我记忆深入的话， 一个Google 工程师写的， 我摘录下来作做为文章的开始，表达我在腾讯这些年和一些出色的工程师合做的过程当中一块儿学习和实践 CodeReview 以后， 对CodeReview 文化的一种由衷的认同和尊敬。

2. Rietveld

我 2008 年进入腾讯， 主要呆在 R2参与soso 的开发， 从 2008 ~ 2010 年我都很不屑于 R2 的研发质量管理，以为作得实在很糟糕：没有统一的编码规范、没有unittest，甚至经历过有些人不会用 svn, 不太懂代码的版本控制， 固然我本身也不怎么懂开发流程中的质量管理， 那时候连 codereview 是什么都没据说过。2010年初， 公司开始在搜索上加大投入， 招聘了一些牛人，尤为是 Google 的一些工程师， 而这些人也把 Google 的一些开发文化带入了 R2。虽然 soso 最终的命运并不如人所愿， 可是在 2010~2013年间， 许多soso 的工程师是学习并经历了什么是严谨而强大的开发流程，也见过咱们曾经山寨的 Google 的技术， 以及山寨的 Google 的开发流程。 而至今为止，咱们和不少同事回忆起来，咱们常调侃说咱们是见过猪跑也吃过猪肉的。

2010年4月从Google 来到咱们部门的一位牛人是 yiwang, 作机器学习的博士， 作了一次 LDA topic modeling 的报告， 使人印象深入， 接触了几回以后，我主动找了 yiwang 一块儿合做，学习 LDA 并参与并行 topic modeling 的开发。yiwang 让我作的第一件事就是研究一下 Google 开源的 Rietveld code review 系统， 并找一台机器把 Rietveld 搭建起来。因而一顿折腾以后， 咱们搭建了公司的第一套严格意义上的 codereview 系统， 固然那个时候也只是供咱们小组内本身使用。 搭建好 Rietveld 以后， 咱们作的第二件事就是把 code style 都统一到 Google 发布的 C++ coding style。 最先在这套系统上进行 codereview 的就是4个工程师 yiwang, leostarzhou, charlieyan, 和我。

全部和 yiwang 合做过的人在 codereview 中都苦不堪言， 他是近乎苛刻的严格执行 Google C++ coding style, 咱们提交的的 code 中多一个空格，多一个空行；或者是注释符号 “//” 后面少一个空格， 全都被打回来修改；更别说变量、函数名命名不符合规范，随意使用变量缩写这些大的禁区了。对于咱们最初的代码， yiwang 挨个细致的指出咱们违反规范的地方， 在 comment 中贴上Google C++ 规范的具体要求的 URL anchor， 以说明咱们为什么违反了规范。 一开始我是有点不觉得然的， 来回几回以后我本身也很差意思了， 把 Google coding style 从头至尾仔仔细细的看上3遍， 认真的记住一些细节， 因而很快就开始上手了。

天然， 后续咱们合做的工程师逐渐多了起来， 对于 codereview 的必要性，许多人是有质疑和挑战的， 因而在和yiwang合做的几年里， 我反复听他说了三个故事。

第一个是关于他本身的，他从小开始 coding, 高中的时候已经拿到工程师的认证，进入 Google 以前 coding 无数， 进入 Google 以后写的第一个100行的程序， 被贴的 comment 超过了100行。 因此不要抱怨咱们的标准严格， 严格的标准才能产出漂亮的代码， 咱们今天在腾讯作的 codereview 真是不算啥。

第二个是关于Google 内部 codereview 的争议的， 听说 Google 内部创建 codereview 制度的时候，也是不少工程师不乐意， 尤为是年长的工程师， 写了多少年代码了， 被一群年轻的小屁孩在代码上指指点点，实在是受不了。 最后说是 Google 的一位极有地位的人物出来讲话了：咱们要想把工程质量控制好， 不管是资历深浅，都必定要遵照共同的代码规范，因而 codereview 制度得以创建。

第三个是解释为什么要用 Google C++ coding style 的， Google 的代码规范是一群顶尖的 coder 制定的， 变量名用小写， 函数名用驼峰式， 那都是有严格的视觉美学讲究的，让人看了很舒服的，甚至是作过视觉对照研究的。虽然我对此存疑, 我仍是认同 Google C++ coding style 是我见过的最好的 coding style。 而咱们腾讯的一位 HR 在和我闲聊的时候也表示过一样的观点 :-)。

Coding style 的规则制定不少都是相对的， 而Google style作得 NB 的地方就是， 他设定的大多数规则的目的他并非要证实本身是正确的， 而是详细的列出各规则的优缺点(pros and cons)，让你更深入的理解这些规则。著名的统计学家 George E.P. Box 在数据建模中说过一句广为流传的话： All models are wrong, but some are useful. 套用这句话到 code review 的 coding style 中， 我想说 All coding rules are wrong, but some are useful。固然 Google style 好的一个额外的缘由是 Google 提供了 cpplint 这个工具自动检查你的 code 中是否有违反规范的地方， 大部分的低级错误都会被检查出来。

3. 推广 CodeReview

咱们小组刚开始把 Google 的 Rietveld 在内部搭建起来并在部门内推广使用之后， 发现了一些问题， 主要包括：

1. issue 提交以后如何经过 email 发送到公司的 outlook 账号
2. Rietveld 提供的提交 issue 的工具 upload.py 对中文的支持很差
3. 底层用的文本文件作存储， 访问量大的时候，提交 issue 多的时候， 速度不行

前面两个问题charlieyan 和我对 Rietveld 的 python 源代码作了一些修改， 很快解决了。 第3个问题咱们一直解决得很差。 不过当时来讲支撑咱们几个小组的开发仍是 OK的。 因此 yiwang 开始继续推进 Rietveld 在 R2 的使用。 到了 2011 年的时候这套工具已经在 R2 的多个部门被使用了。 下面一张图是咱们曾经统计过的各个部门当时的使用状况

2010 年咱们在推广 codereview 的时候， yiwang 也联系了研发管理部的同事， 也就是 code.oa.com 的维护人员， 当时 code.oa.com 并无严格意义的 codereview 功能。 它提供一个代码 checkin 以后进行 codereview 的功能， 因此也没有发送 codereview 的 upload.py 脚本；这个功能天然是有用的，可是过后的 review 每每有缺陷， 很难替代事前的 review。事实上更重要的问题是， 公司内部当时并无造成 codereview 的文化。 yiwang 和研发管理部的人交流的时候强烈的推荐了 Rietveld 系统， 不久以后code.oa.com 也仿照 Rietveld 开发了正式的 codereview 功能，而用于提交issue 的 tcr.py 也正是基于Rietveld 的 upload.py 作的修改。

而在搜搜内部，情况则有些不一样。 Google 来的朱会灿带领的搜索云平台(搜索基础架构部）对 codereview 文化建设作了强力的推进， 基础架构部门的几个牛人开始使用这套系统以后， wujie, phong 他们接管了这套 CodeReview 系统， 当时上面提到的第三个问题当时已经成为了一个严重的问题， wujie， phong 开始改进 Rietveld 的性能问题， 把底层的文本文件存储替换为 mysql 数据库， 使用 apache 服务器替换了 Rietvled 基于 Django 的 HttpServer, 因此Rietveld 第三个和性能相关的问题也完全被解决了。 后续 huanyu 申请了 codereview.soso.oa.com 这个域名专门提供 codereview 服务， 因而直到搜搜和搜狗合并， 搜搜内部的好几个部门的兄弟们一直基于 Rietveld 系统作 codereview。

4. C++ Readability

搜索广告部门的情境广告中心应该是我经历的 code review 文化创建得最完全完善的地方。 当时 yiwang 是这个中心的总监，我和 huan 各负责一个小组， 咱们中心要负责开发一套 AFC(ads for content) 的情境广告产品， 和如今广点通有不少类似之处。 huan, yiwang, 包括咱们的 GM paulyan 都是 Google 的工程师， 而 AFC 这个产品几乎是从零开始开发， 因此很天然的， 许多开发流程都是山寨的 Google 的， 虽然 AFC 这个产品最终因为各类缘由并无在公司内部成功， 可是许多参与开发的工程师对于咱们当时创建起来的开发流程应该是印象深入的。 固然 codereview 是其中很重要的一个环节。 Huan 借鉴 Google 的CodeReview 流程， 在中心推进了一个 C++ readability 制度的创建。 一个语言的 readability 表示工程师对这个语言和相应的编码规范的熟悉， 这个 C++ readability 最初只授予有限几个高职级的工程师。 每个 codereview issue 至少须要获得一位具备 C++ readabiltiy 的工程师的承认， 这个 issue 才可以提交到代码库中。

而每个没有 C++ readability 的工程师要想得到这个 readability, 须要提交一个 codereview issue 作专门的申请， 这个issue 的标题必须包含 “Apply for C++ Readability”, issue 至少包含三个文件， xxx.cc, xxx.h, xxx_test.cc, 若是这个 issue 经过严格的 codereview 被经过了， 就能够授予这个工程师 C++ readability 。 而一般这种申请的 review 过程你们都会特别的认真细致。

5. 在广点通 CodeReview

2013年10月， 搜索广告平台部并入广点通成立了SNG 的效果广告平台部， 原来AFC 的系统因为这个合并事实上基本废弃， 只是在开发过程当中不少模块组件被逐步迁移到了广点通的系统上。 咱们刚进入广点通的时候， 广点通使用 code.oa.com 平台作 codereview, 也创建了一些 codereview 的文化， 可是有不少地方是执行不到位的。 coding style 虽然也推崇 Google coding style, 可是执行上不严格， 因此整个代码库的代码风格是很不统一的。 这也给咱们的 codereview 执行带来很大的挑战：

• 旧的代码风格不统一，咱们如何统一代码风格， 旧的代码是否须要基于新的风格重写？
• 整个产品都在高速的开发迭代中，老大对于产品功能的开发演进有明确的指望， 产品经理每天在开发人员后面催进度， 咱们是否要执行严格的 code review ？

广点通最后仍是明确的规定 coding style 统一到 Google C++ coding style， 咱们团队的 CodeReview 也总体转战 code.oa.com, 咱们本身再也不使用 Rietveld（不过这套系统并无被废弃， wujie 2014年一月的时候把这套系统搭建起来服务于微信， 并申请了域名 cr.oa.com), code.oa.com 的 CodeReview 功能通过几年的改进， 也确实变得很好用。 考虑到产品迭代的节奏， 咱们广告质量中心在推动的过程是渐进的：

• 全部新提交的文件必须使用 google style
• 全部新修改的代码必须使用 google style
• 在指定的时间范围内， 重要模块的 code 必须重构为 google style

为了让 codereview 文化在广点通能更加完全的创建起来、执行到位， 几位总监和架构师也迅速推进 code.oa.com 中实现了 code owner 的机制， 每一个目录下面明确设置 code owner, 每一个change issue 要想提交， 必须有 code owner 经过才能够。 而刚开始执行的时候， 广告质量中心为了保证 code review 执行到位， code review 是很是集权的， 全部相关目录的 owner 只写上总监。因此质量中心发出的全部 code review issue 只有总监经过了以后才能够提交， 在必定的时间内这确实影响开发效率， 不过对教育全部开发人员的 code review 意识是高效的。 在经历了近三周的严格控制以后， 目录的owner 才加上组长， 而后逐步放开到组内的一些骨干成员。

对于咱们小组而言， 有一半是以前 AFC的开发人员， 经历过严格的 codereview 训练； 有一半是广点通原来的开发人员，没有经历过严格的 codereview 训练。 为了教育你们的 codereview 意识，在小组的周会上我要反复的强调 codereview 的重要性；同时我明确的指出每一个人的考评是和 codereview 的表现相关的。

对于广告业务的工程输出主要包括项目中的设计文档， 代码实现， 线上A/B test实验和实验结果跟进分析， 以及平常在小组内的分享(日常积极的邮件讨论，wiki 建设， 组内的 techtalk 分享等)。 全部的一线工程师， 不管职级高低，最重要的工程输出原则仍是 show me the code, 而 codereview 是最可以反应这个客观输出的。 在小组内部， 咱们尽可能让每一个人的 codereview 参与情况都公开透明， 因此咱们要求

• 每一个 codereview issue 要明确发送到你的项目合做者， 项目合做者 review 以后才容许提交
• 每一个 issue 都必须全员转发到小组内全部成员， 小组内的任何人只要愿意，均可以去review 其余人的代码

因为全部的 codereview issue 都会发送到小组内全部成员， 因此我是可以在 code.oa.com 上看到全部人的 code 产出状况的。我经过给你们作 codereview 判断小组每一个工程师的工程输出， code.oa.com 上大体是能够看到小组内每一个人参与 codereview 的程度的， 包括他发出的 issue, 他给组内成员作的 codereview comments 等等。 code.oa.com 上提供了一个功能， 能够 dump 出全部人的 codereview issue, 因此考评的时候我会检查每一个成员在半年以内的 codereview 输出情况，检查组内成员提交的 issue 的质量。

不过 code.oa.com 上查看每一个人的 codereview 参与情况的功能如今仍是太弱， 若是code.oa.com 能提供一个统计分析功能, 使得每一个人能够看到本身的一些统计指标， 包括

• 我修改的文件总数，修改和提交的代码总行数
• 我给合做者作了多少次 codereview, 提供了多少次 comments
• 合做者给我作了多少次 comments

若是小组的组长可以看到组内成员的这些统计指标，无疑对于考评每个小组成员的工程输出会提供不少客观公正的信息。

6. CodeReview 感悟

作了4年的 CodeReview, 基本上这个流程在咱们的团队已经深刻骨髓，成为了小组工程工做中的种行为习惯。 若是有一天我离开了腾讯， 我相信我在腾讯所经历的相对严格的 CodeReview 训练会帮我作好下一个项目的工程质量管理。 为什么要作 CodeReview 网络上有不少的论证， 以前和 yiwang 合做的时候， 团队内总结的几点以下(主要都是 yiwang 总结的):

• Code Review保证开发质量，整个过程有章可循，无需顾及“面子”，可以修正不少平时不注意或者明知故犯的小毛病
• Code Review高效沟通交流，不需电话和视频会议，让北京深圳两地的同事们一块儿协同工做
• Code Review帮助知识传递，团队内细粒度的知识分享， 你们一块儿逐行的点评代码比用技术交流会以及KM投稿更细致
• Code Review保证代码的可持续使用，代码风格不一致致使一个系统每次移交负责人都被重写一次
• Code Review和敏捷开发相容，而不是相斥，敏捷不是片面追求发布次数，而是保证质量的发布
• CodeReview 是一种社交的途径， 鼓励小组成员之间多作技术交流
• CodeReview 对新人的成长极其有利。 不少新人第一次作 codereview 的时候感受都是惨不忍睹， 被拍晕了， 我见过的最惨的 issue 被要求修改了20屡次以后才被容许提交。然而这种方式对新人的成长很是高效，一个coding 能力不错的工程师， 几个 codereview issue 以后， 很快就能写出合格的代码。
• CodeReview 增长团队内代码的可维护性, 同一份代码至少会有两我的熟悉——代码的做者和审阅者。

如何建设一个团队的 code review 文化， 跟随 Google 的脚步， 也有一些具体的意见：

• 须要培养合格的reviewer。 创建各类编程语言的readability认证机制，从一组“种子”reviewer开始，让更多同事得到认证成为合格的reviewers
• 须要细化和公开各类语言的code style。 评注中能够给出建议的出处的URL，使得保持代码风格和可维护性落到实处
• 须要创建Code Lab机制。 帮助工程师们入门开发流程和规范
• 须要创建change approval机制。 鼓励组内更多工程师改进代码（敏捷），可是修改在提交前除了必须经过review，还须要相关责任人的approval；在加速开发滚动的同时，保证权责分明
• 引导轻量 review。咱们都经历过有些工程师发一个 change issue 的时候， 几十个文件一并发出来， 一个 issue 多是累积一周的修改， reviewer 看到这种 issue 几乎都是崩溃的，不可能保证 review 质量。 因此好的 issue 应该是轻量的， 大的 issue 应该拆分为小的 issue 发送。下图是 Cisco 公司的codereview 报告中摘出的， 若是一个 issue 的修改代码行数超过 400， 基本上 reviewer 是不可能认真 review 帮你找出 bug 的 。

• 明确issue 可否提交是工程师本身的责任。开发人员有时候抱怨别人 review 得慢， 拖延时间。 咱们小组内部一般明确指出：一个 issue 可否提交是开发人员本身的责任。 开发人员本身要积极推进合做人员帮忙 review issue， 推进 issue 被经过并及时提交。

7. 结语

学术界的全部高质量论文都是 peer review 以后才有可能发表的。 写过 paper 的硕士博士都有一个体会， 接到的各类 review 意见有时候令你很是的痛苦， 然而不管是你拒绝了 reviewer 的意见，仍是接受意见修改了文章， 你都会经历一个认真思考的过程去提高你文章的质量。 高质量的论文是科研人员严格 review 出来的， 我相信高质量的 code 也能够经过工程师的 review 产生。 把CodeReview 打形成团队的习惯，而咱们逐渐会发现这种习惯所释放出来的正能量。 但愿有一天, 咱们也能作到：

At Tencent, no code, for any product, for any project, gets checked in until it gets a positive review。