时间过得很快，转眼间 Code Review 机制在丁香医生前端团队已经运作一年多了。今年4月初时，将团队在 Code Review 方面的一些经验在丁香园前端团队进行了分享，各个业务线的前端同学们逐步开始尝试 Code Review 机制，目前也有了一定的收获。是时候将这些实践经验落实到文字上，来和更多的朋友们进行交流了。
起因
世上没有无缘无故的爱，也没有无缘无故的恨。同样，也没有无缘无故的 Code Review。最开始时，丁香医生前端有2个人，基本上是1人在做丁香医生 SPA 项目，1人做丁香医生管理后台项目。
将时间点放到17年初，团队从2个人变为了3个人，此时主要有三个前端项目（丁香医生 SPA、丁香医生管理后台以及丁香医生 Hybrid App）在迭代，其中主要是 SPA 项目会涉及到三个人的交叉维护。这个阶段便会开始暴露出一些小问题。比如：
- 编码风格不一致
- 有些他人写的业务逻辑，在交叉维护时，需要花更多的时间上手
- 一些低级的 bug 在代码部署到测试环境才被发现
为了解决这些问题，我们决定开始尝试 Code Review。项目的代码是托管在公司内网的 Gitlab 上的，于是我们会开始摸索着基于 GitLab 中项目的 Merge Request 进行他人代码的 Code Review。
17年 Q2 时，我们开始频繁的迭代丁香医生小程序，同时运营团队也会开始提出一些运营类H5的需求。团队成员有4人了。随着新鲜血液的加入，我们遇到了新的问题：
- 新人的加入提高了团队代码风格的差异性
- 在不是很了解现有项目的基础上，实现的新功能代码会产生冗余
- 谁来为新成员的代码质量和成长负责？（注意：这是重要的一点）
此时我们依旧在做 Code Review，但实际上并没有严格的去执行，也没有一个关于 Code Review 的标准供大家遵守。
毫无疑问的一点：随着丁香医生业务的发展，这些问题是需要被解决的，否则长远来看无论是对于团队还是团队成员，都是有较大伤害的。
17 年 Q3 时，团队已经有 6 个人了。每加入一个新人，上述问题的复杂度就会增加一些。为了解决这些问题，团队决定将 Code Review 作为一项基本制度，严格去执行。
如何去做 Code Review？
前提
在开始严格的去做 Code Review 之前，我们确定了三点基础规范。
- 基于项目版本控制，统一项目遵守的 Git 分支模型
- 对于 JavaScript，使用统一的 Eslint 规则
- 结合团队成员现有风格，明确统一的代码规范
工具
使用的工具就地取材，依旧是 GitLab。整个 Code Review 流程在 GitLab 项目中有两个点比较关键：Merge Request（简称：MR）、Discussion（简称：Diss）。
在这两点基础上，我们确定了几个角色：
- Owner（需求负责人，代码改动提交者，MR 发起者）
- Reviewers（MR 参与者，前端团队的同事，可能不止一个人。负责 Review 代码。）
- Disser（某个 Reviewer。对某个 MR 发起 Discussion 的人。）
流程
- 对 GitLab 上需要进行 Code Review 的项目进行设置（Settings - General - Merge request settings - Only allow merge requests to be merged if all discussions are resolved）。
- Owner 在本地开发环境，某分支（以某功能分支 feat-example 为例）做好功能开发，充分自测后将代码推送到 GitLab。
- Owner 基于 feat-example 分支，发起目标分支为 develop 分支的 MR。MR 需要有尽可能详细的描述。比如：需求文档地址，做了哪些修改，某个功能的设计实现思路，需要哪几位 reviewer 对本次 MR 进行 Code Review 等。推荐使用 MR 模板。
- Owner 成功发起 MR 后，通过团队协作工作告知 Reviewers 有 MR 需要进行 Code Review，以及 MR 的紧急程度。
- Reviewers 基于 MR 进行进行 code review。如果对 MR 有任何问题，在 GitLab 上针对具体代码进行 comment（发起 Discussion），review 完成后通知 Owner 结果（本次 MR 通过 / 本次 MR 有 n 个 Diss）。如果有 Diss，Owner 需要对每一个 Diss 进行回复，直至所有 Diss 的状态变更为 Resolved。
- Owner 对 MR 进行 merge 操作，并在测试环境发布代码，通知相关 QA 同学测试，QA 测试通过后由 QA 通知产品和设计师进行验收。（此处有一个细节：Owner 如果确定可以进行 merge 操作？我们想到有两个方案：1. 以 Reviewers 通知 Owner 为准 2. 以 Reviewers 给 MR 点赞为准，因为 GitLab 上是可以对 MR 进行点赞操作的。目前团队采用的是第2种方式。）
- 如果测试或者验收环节发现问题，Owner 需要对代码进行修改，然后发起新一轮的 MR，直至测试环境代码通过验收。
- 和 QA 同学确认代码可以发布至生产环境，并进行代码发布，通知 case 相关同学某功能已上线。
原则
在执行 Code Review 过程中，我们有一些原则需要遵守：
- Owner 发起 MR 之前的代码需要进行充分自测
- 代码版本控制 commit 的粒度不要太大
- 不阻塞他人的工作，尽快响应他人的 Code Review 请求（这一点比较考验团队成员的合作精神、团队意识。同时也要求开发者要合理安排自己的时间，要有能力随时放下手中的工作，随时继续手中的工作）
- 如果某个 MR 紧急，可以告知 Reviewers
- 除有必要，否则 Owner 不要在提测验收阶段删除分支（例如勾选“remove source branch when merge request is accepted.”），应等待分支合入master分支后移除，避免预发/测试分支重建时被遗漏。
- 定期回顾和总结 Code Review 执行情况（比如在团队周会时进行）
边界
清楚了 Code Review 流程之后，其实还有一些边界情况需要考虑。我会将团队目前采用的处理策略写出来供参考。
- 周末出现线上紧急 bug 要遵循 Code Review 流程吗？可以不进行 Code Review，以快速修复 bug 为主。
- 某个需求（项目）留给开发时间非常紧张时怎么办？可以不进行 Code Review，优先保证按时需求（项目）上线。
- 团队内部项目、组内同学个人发起的兴趣项目是否需要进行 code review？决定权在项目 Owner。
- MR 遇到代码冲突怎么办？建议在 code review 之后，由 Owner 将代码拉取到本地进行 merge 并解决冲突，然后将最新代码推送到 GitLab（此时 GitLab 上 MR 会自动 merge 掉）。
收获
坦言，在一个从未进行过 Code Review 的团队想把这个机制运作起来，并不是一件容易的事情。尤其是在决定开始进行 Code Review 后的起步阶段。但是如果能认准方向，团队的成员齐心协力朝着既定的方向去走，最终会获得如下的收获的：
- 团队成员代码风格统一
- 减小了项目交叉维护的阻力
- 使新成员更快速融入团队
- 避免了低级 bug 在测试环境出现
- 良好的技术交流氛围
待完善
上面描述的这个机制并不是完美的。目前我可以想到的可以优化的点如下：
- 优化编码规范（技术本身在发展，团队成员的水平在提高，随之之前定下来的编码规范也会适当的进行优化）
- Check List（这一点实际上目前团队已经开始做了。当业务具有一定复杂度后，某些业务逻辑的迭代难免会牵扯较多已有业务，此时如果有一份 Check List，会帮助 Owner 及 Reviewers 更好的进行 Code Review）
- 激励机制
- 代码测试用例（主要是指业务代码增加测试用例。目前团队也开始进行了一些尝试。）
- 自动化
写在最后
将团队在使用的 Code Review 机制以文字的形式沉淀下来，主要是想分享给更多的人。如果这些文字对某些人、某些团队有帮助，那对于我来说是一件令人欣慰的事情。如果能接收到关于优化现有机制的指点，也会是一件令人开心和感激的事情。
此外，还想表达的一点是：丁香医生前端团队是一个非常在意每一个团队成员成长的团队。
我猜，你可能猜到接下来我要说什么了。
是的，随着丁香医生业务的发展，我们需要优秀的前端同学加入我们，一起茁壮肆意成长。更多关于团队的介绍，可以参考请问丁香医生前端团队怎么样？
作者：丁香园F2E
链接：https://juejin.im/post/5b97d684e51d450ea363092f
来源：掘金
著作权归作者所有。商业转载请联系作者获得授权，非商业转载请注明出处。
