从零开始 Code Review，两年实战经验分享！

程序员的成长之路

互联网/程序员/成长/职场

阅读本文大概须要 7.4 分钟。javascript

重要通知：文末有周末中奖的名单，中奖的小伙伴尽快添加我微信。css

前几天看了《Code Review 程序员的寄望与哀伤》，想到咱们团队开展 Code Review 也有 2 年了，结果还算比较满意，有些经验应该能够和你们一块儿分享、探讨。
html

咱们为何要推行 Code Review 呢？咱们当时面临着代码混乱、 Bug 频出的情况。前端

当时我以为要有所改变，但愿能提升产品的代码质量，改善开发团队面临的困境。而且我我的在开发上有不少经验，也但愿这些知识可以在团队内传播。java

各类考虑后，咱们最后认为推行 Code Review 能改善或解决咱们面临的不少问题。程序员

这篇文章的目的不是告诉你们怎么在一个团队内推行 Code Review，首先由于我我的仅在一家公司内推行过，并无不少经验。web

其次每家公司、每一个团队的状况都不太同样，应该根据公司或团队的实际状况选择恰当的方案，并根据成员的反馈来及时调整，推进 Code Review 的实施。面试

因此，本文是介绍咱们公司是如何实施 Code Review 的，咱们是如何解决咱们遇到的问题的，但愿咱们的经验能给你们带来些帮助。算法

行文仓促，若有遗漏或错误，欢迎指正。数据库

▌1、流程和规则

通过简单的对比、试用，咱们最后采用了 Git Flow+Pull Request（PR）模式来作 Code Review。（PR模式详情可参见 Git 工做流指南：Pull Request 工做流）

Pull Request(PR) 简单的说就是你没有权限往一个特定的仓库或分支提交代码，你请求有权限的人把你提交的代码从你的仓库或分支合并到指定的仓库或分支。

因为 PR 须要有权限的人确认，因此很是适合在这个过程当中作 Code Review，是否接受或者拒绝就取决于 Code Review 的结果。

在支持 PR 模式的软件里，每个 PR 都有一个新增代码的对比（diff）界面。

代码审核者能够在线浏览请求合并的新增代码，并针对有疑问的代码行添加评论，经过这种方式来实现 Code Review。

评论能够被全部有权限查看仓库的人看到，每一个人均可以回复任何人的评论，有点像论坛里某个帖子的讨论。

这种模式是过后审核，也就是代码已经提交到了中心仓库，Review 过程当中频繁的改动会形成历史签入记录的混乱。

固然 Git 能够采用更改历史记录来解决这个问题，因为容易误操做，咱们通常只在基础类库这类要求比较严格的项目上实施。

咱们所了解到的支持 PR 模式的软件都采用 Git 做为源代码版本控制工具，因此咱们的源代码版本控制工具也迁移到了 Git。

因为 Git 太灵活了，所以诞生了不少的 Git 流程，用来规范 Git 的使用。

常见的有集中式工做流、功能分支工做流、Gitflow 工做流、Forking 工做流、Github 工做流。

咱们对 Git Flow 作了些调整，调整后的流程被命名为 Baza Flow，定义见后文。

根据 Baza Flow，咱们大部分仓库只定义了 2 个主干分支，master 和 develop。(例外，咱们有一个仓库有 3 个开发小组同时进行开发，定义了 4 个主干分支，目前还比较顺畅，再多估计主干分支之间的合并就比较繁琐了。)

master 对应生产环境代码，全部面向生产环境的发布来源都是 master 分支的代码。develop 则对应本地测试环境的代码。

绝大多数状况下，QA（测试）只测试 develop 分支和 master 分支的代码。

因为开发人员都在一个团队内，因此咱们没有采用基于仓库的 PR，采用的是基于分支的 PR。

咱们对主干分支的操做权限作了限制，只有特定的人才能操做， develop 分支是项目开发 Leader 和架构师，master 分支是 QA。

有权限往主干分支合并的成员会按照约定的规则来执行合并，不会合并无完成审核的 PR。

上面这点其实蛮重要的，因此咱们会对有权限合并的人有特别的约定，在什么状况下才能合并代码。（见后文 PR 的说明）

PR 的发起人要主动的推进PR的审核，Leader 也会密切关注PR审核的进度，在须要的时候及时介入。

咱们配置了 CI 服务器（什么是CI）只编译特定的分支，一般是 develop 和 master 分支。

全部的代码合并到了主干分支以后，都会自动触发编译和本地测试环境的发布，QA 无需依赖开发人员编译的代码来测试，也无需本身手工操做这些，保证了开发人员和测试人员的相互独立。

咱们本地测试环境的发布包含了数据库和站点的发布，全自动的，发布完成之后就是一个可用的产品，有时间这部分也能够分享一下。

咱们还使用了 Scrum 里面一个很重要的概念：完成定义。

就是咱们规定了咱们一个任务的完成被定义为：代码编写完成，通过自测，提交的 PR 通过审核而且合并到主干分支。

也就是说，全部的代码被合并到了主干分支以后任务才算是完成，而被合并到主干分支必需要通过 Code Review，这是强制的。

因为咱们的托管软件对于 Pull Request 的限制，咱们对 Git Flow 作了改动，改动的地方有：

一、每个大功能咱们会建立一个单独的 feature 分支，项目开发人员基于这个单独的 feature 分支建立本身的任务分支。

好比，对于 CS 2 项目来讲，启动的时候分支的建立是：master -> develop -> feature/v2。

开发人员应该基于这个大特性分支 feature/v2 来建立本身的任务分支，好比建立 XXXX，能够用一个单独的分支 feature/v2-xxxx。

完成这个任务之后，当即向上游分支（feature/v2）提交 pull request。而后从 feature/v2-xxxx 建立本身的下一个任务分支，好比 YYYY 编辑 feature/v2-yyyy。

请注意，合并到上游分支的功能必须相对独立并且是可用的，分支任务工做量 0.5-1 个工做日，不宜超过 2 个工做日，超过 2 个工做日不向上游合并，须要向团队解释。

代码通过 Review 之后，可能会进行必要的修改，修改在原分支修改，修改完毕代码合并进上游分支，原分支会按期删除。

项目组成员在收到合并成功的通知后，请自行从上游大特性分支向下合并到本身当前的开发分支。

提交 pull request 后建立新任务分支的时候务必知会一下相关配合同事（好比前端的同事），让他们在新的分支上继续开发。

二、对于小功能，预计在 0.5-1 个（不超过 2 个）工做日工做量的开发任务，直接基于 develop 分支建立特性分支便可。

三、在各个分支遇到的 bug，请基于该分支建立一个 Bug 分支。

若是在缺陷跟踪管理系统上有对应的项，命名请使用缺陷跟踪管理系统的 ID，好比 BAZABUG-1354 好比这个 Bug 的分支命名就是 bugfix/BAZABUG-1354。

若是在缺陷跟踪管理系统上没有对应的项，命名请简短的说明修改内容，好比 “JX 9df2b01 引用 bootstrap css 虚拟路径重写，避免出现字体没法找到的问题”，分支命名能够是 bugfix/miss-font。

完成修改之后提交并推送到中心仓库而后当即向上游分支提交 pull request。

四、发起 pull request 之后，请将 pull request 的连接在 IM 上发给代码审核者，以此通知对方及时进行审核。

▌2、执行

咱们在团队内部提倡质量优先，开发团队不能为了进度牺牲质量，并在团队内部达成了共识。

因此，不管进度有多么紧迫，Code Review 的过程都必定会作。

全部的问题必定会被提出，只是会根据进度的紧迫程度，以及问题的大小，改动成本，决定问题是如今解决，仍是加一个 TODO，并记录在缺陷跟踪管理系统内，以防往后遗忘。

多数状况下，咱们都会要求当即解决，哪怕所以形成了发布的推迟。

咱们深知，其实多数状况下，如今不解决，往后不知道猴年马月才能解决。

咱们在团队内推行 Code Review 的过程当中没有遇到太多阻力。

缘由大概有两点，首先管理层方面了解以前遇到的各类问题，也迫切但愿能有所改善，因此从一开始就是支持的态度。

其次，绝大部分开发人员以为在这个过程当中能本身能学习到东西，并无抵触，遇到很好的意见时你们都仍是很高兴的。

最后，慢慢的造成了一种氛围，整个团队都会自觉的维护它。

附一张咱们审核的对话图，这位童鞋尝试对系统内部散落各地发业务邮件的代码作一个整理，用一套模式来处理，调整了 3 版才定调，而后修改了不少细节才经过了合并，先后大概用一个多星期时间：

表面上看来 Code Review 会延缓项目的进度，可是在咱们 2 年多的执行过程当中，大多数时候没感受到有延缓。

缘由是，虽然代码合并的周期变长了，可是因为代码质量提升了，致使 Bug 变少了，因为 Bug 引发的返工问题也变少了，所以总体的进度其实并无延缓。

我我的认为对一个成熟的团队其实作 Code Review 反而会加快总体的项目进度，可是手头上没有统计数据支撑个人观点。（对于软件开发的度量，欢迎有心得的同窗告知我）

咱们每一个分支有权限合并的人都不止一个，这样能够保证有人请假不在的时候，代码仍然能够被其余同事审核经过以后合并。

半年前，咱们团队加入了不少新成员，刚加入的新同事对规范、项目、产品的熟悉程度都不高，致使了有一段时间，咱们遇到了 PR 审核周期变长的问题。

加上以前遇到的一些问题，咱们总结了一个说明，目的是减轻 Code Review 对开发人员工做的负担，加快 PR 审核经过的过程。

说明以下：

Pull Request 的说明

任务完成才能提交 PR。

PR 应该在一个工做日内被合并或者被拒绝。

PR 在有严重问题（包括但不限于架构问题、安全问题、设计问题），太多问题，或者任务无效的状况下会被拒绝。

严禁一个PR里面有多个任务，除非它们是紧密关联的。

PR 提交以后只容许针对 Review 发现问题再次提交代码，除非有充足的理由，严禁在同一个 PR 中再次提交其它任务的代码。

提交PR时候有一个描述框，内容会自动根据 Commit 的 message 合并而成。

切记，若是一次提交的内容包含不少 Commit，请不要使用自动生成的描述。

请用简短可是足够说明问题的语言（理想是控制在3句话以内）来描述：

你改动了什么，解决了什么问题，须要代码审查的人留意那些影响比较大的改动。

特别须要留意，若是对基础、公共的组件进行了改动，必定要另起一行特别说明。

审核人员邀请原则：

在建立 PR 时，Reviewers（审核人）一栏里主要填写“必需审核人”。只有这些人审核都经过，才容许合并。

除了“必需审核人”外，还有一些其它审核人，咱们能够在 Description 里作为“邀请审核嘉宾”@进来。

主干分支间的合并，如 Develop => Master，或Master => Develop 等，则须要把整个团队（开发+QA）都列为“必需审核人”。

必须审核人的列表由团队决定，可能包括如下人选：

团队 Leader

前端架构师（若是有前端代码改动）（能够受权）

后端架构师（若是有后端代码改动）（能够受权）

产品架构师

对此 PR 解决的问题比较熟悉的（以前一直负责这部分业务的同事）

此PR解决的问题对他影响比较大（好比认领的任务依赖此PR的同事）

其它审核人，包括但不限于：

须要知悉此处代码改动的人但又没必要非要其审核经过的同事。

能够从这个 PR 中学习的同事。

能够受权指的是，根据约定，Bug 修复之类的改动，或者影响较小的改动，前端架构师和后端架构师能够受权团队内的某个资深开发人员，由这个资深开发人员表明他们进行审核。

主干分支之间的合并，大型 Feature 的合并，前端架构师和后端架构师须要参与。

上述审核人关注的视角不太同样：

团队 Leader 关注你是否完成了任务，先后端架构师关注是否符合公司统一的架构、风格、质量，产品架构师从整个产品层面来关注这个 PR。

熟悉此问题的同事能够更好的保证问题被解决，确保没有引入新问题。

被影响的同事能够及时了解他受到的影响。

团队 Leader 或者产品架构师若是以为 PR 邀请的审核者不足或者过多，必须调整为合适的人员，其它同事能够在评论中建议。