我们是怎么做Code Review的

1、我们为什么要推行Code Review呢？我们当时面临着代码混乱、Bug频出的状况。

4、一、流程和规则经过简单的对比、试用，我们最后采用了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，这是强制的。

6、审核人员邀请原则：1.在创建PR时，Reviewers（审核人）一栏里主要填写“必需审核人”。只有这些人审核都通过，才允许合并。2.除了“必需审核人”外，还有一些其它审核人，我们可以在Description里做为“邀请审核嘉宾”@进来。3.主干分支间的合并，如Develop=>Master，或Master=>Develop等，则需要把整个团队（开发+QA）都列为“必需审核人”。必须审核人的列表由团队决定，可能包括以下人选：团队Leader前端架构师（如果有前端代码改动） （可以授权）后端架构师（如果有后端代码改动） （可以授权）产品架构师对此PR解决的问题比较熟悉的（之前一直负责这部分业务的同事）此PR解决的问题对他影响比较大（比如认领的任务依赖此PR的同事）其它审核人，包括但不限于：需要知悉此处代码改动的人但又不必非要其审核通过的同事可以从这个PR中学习的同事可以授权指的是，根据约定，Bug修复之类的改动，或者影响较小的改动，前端架构师和后端架构师可以授权团队内的某个资深开发人员，由这个资深开发人员代表他们进行审核。主干分支之间的合并，大型Feature的合并，前端架构师和后端架构师需要参与。上述审核人关注的视角不太一样：团队Leader关注你是否完成了任务，前后端架构师关注是否符合公司统一的架构、风格、质量，产品架构师从整个产品层面来关注这个PR。熟悉此问题的同事可以更好的保证问题被解决，确保没有引入新问题。被影响的同事可以及时了解他受到的影响。团队Leader或者产品架构师如果觉得PR邀请的审核者不足或者过多，必须调整为合适的人员，其它同事可以在评论中建议。三、收获我们团队实施Code Review收获不少，总结出来大概有以下几点：1、短期内迅速提高了代码质量。 原因有几个，大家知道自己的代码会被人审核之后写得会比较认真。 理论上代码质量是由整个团队内最优秀的那个人决定的。 大家也能在Review的过程中学习到其它同事优秀的编码。2、Bug数量迅速减少。 但是这个我们没有数据统计比较，比较遗憾。 我和QA聊过，他给我的数据是在我们的一个新项目每2周一次的大发布，平均只会发现1~2个Bug。 这点提高了整个团队的幸福感，大家不用经常被火烧眉毛。3、团队成员对项目的熟悉程度会比较均衡。 新同事通过参与Code Review能很快熟悉团队的规范。 代码不会只有个别人了解、熟悉，Bug谁都能改，新功能谁都能做。 对公司来说避免了人员的风险，对个人来说比较轻松（谁都能来帮你），可以选自己喜欢的任务做。