Code Review 在丁香医生前端团队的实践

时间过得很快，转眼间 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 以前，咱们肯定了三点基础规范。

1. 基于项目版本控制，统一项目遵照的 Git 分支模型
2. 对于 JavaScript，使用统一的 Eslint 规则
3. 结合团队成员现有风格，明确统一的代码规范

工具

使用的工具就地取材，依旧是 GitLab。整个 Code Review 流程在 GitLab 项目中有两个点比较关键：Merge Request（简称：MR）、Discussion（简称：Diss）。

在这两点基础上，咱们肯定了几个角色：

• Owner（需求负责人，代码改动提交者，MR 发起者）
• Reviewers（MR 参与者，前端团队的同事，可能不止一我的。负责 Review 代码。）
• Disser（某个 Reviewer。对某个 MR 发起 Discussion 的人。）

流程

0. 对 GitLab 上须要进行 Code Review 的项目进行设置（Settings - General - Merge request settings - Only allow merge requests to be merged if all discussions are resolved）。
1. Owner 在本地开发环境，某分支（以某功能分支 feat-example 为例）作好功能开发，充分自测后将代码推送到 GitLab。
2. Owner 基于 feat-example 分支，发起目标分支为 develop 分支的 MR。MR 须要有尽量详细的描述。好比：需求文档地址，作了哪些修改，某个功能的设计实现思路，须要哪几位 reviewer 对本次 MR 进行 Code Review 等。推荐使用 MR 模板。
3. Owner 成功发起 MR 后，经过团队协做工做告知 Reviewers 有 MR 须要进行 Code Review，以及 MR 的紧急程度。
4. Reviewers 基于 MR 进行进行 code review。若是对 MR 有任何问题，在 GitLab 上针对具体代码进行 comment（发起 Discussion），review 完成后通知 Owner 结果（本次 MR 经过 / 本次 MR 有 n 个 Diss）。若是有 Diss，Owner 须要对每个 Diss 进行回复，直至全部 Diss 的状态变动为 Resolved。
5. Owner 对 MR 进行 merge 操做，并在测试环境发布代码，通知相关 QA 同窗测试，QA 测试经过后由 QA 通知产品和设计师进行验收。（此处有一个细节：Owner 若是肯定能够进行 merge 操做？咱们想到有两个方案：1. 以 Reviewers 通知 Owner 为准 2. 以 Reviewers 给 MR 点赞为准，由于 GitLab 上是能够对 MR 进行点赞操做的。目前团队采用的是第2种方式。）
6. 若是测试或者验收环节发现问题，Owner 须要对代码进行修改，而后发起新一轮的 MR，直至测试环境代码经过验收。
7. 和 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 流程以后，其实还有一些边界状况须要考虑。我会将团队目前采用的处理策略写出来供参考。

1. 周末出现线上紧急 bug 要遵循 Code Review 流程吗？能够不进行 Code Review，以快速修复 bug 为主。
2. 某个需求（项目）留给开发时间很是紧张时怎么办？能够不进行 Code Review，优先保证按时需求（项目）上线。
3. 团队内部项目、组内同窗我的发起的兴趣项目是否须要进行 code review？决定权在项目 Owner。
4. MR 遇到代码冲突怎么办？建议在 code review 以后，由 Owner 将代码拉取到本地进行 merge 并解决冲突，而后将最新代码推送到 GitLab（此时 GitLab 上 MR 会自动 merge 掉）。

收获

坦言，在一个从未进行过 Code Review 的团队想把这个机制运做起来，并非一件容易的事情。尤为是在决定开始进行 Code Review 后的起步阶段。可是若是能认准方向，团队的成员齐心合力朝着既定的方向去走，最终会得到以下的收获的：

• 团队成员代码风格统一
• 减少了项目交叉维护的阻力
• 使新成员更快速融入团队
• 避免了低级 bug 在测试环境出现
• 良好的技术交流氛围

待完善

上面描述的这个机制并非完美的。目前我能够想到的能够优化的点以下：

• 优化编码规范（技术自己在发展，团队成员的水平在提升，随之以前定下来的编码规范也会适当的进行优化）
• Check List（这一点实际上目前团队已经开始作了。当业务具备必定复杂度后，某些业务逻辑的迭代不免会牵扯较多已有业务，此时若是有一份 Check List，会帮助 Owner 及 Reviewers 更好的进行 Code Review）
• 激励机制
• 代码测试用例（主要是指业务代码增长测试用例。目前团队也开始进行了一些尝试。）
• 自动化

写在最后

将团队在使用的 Code Review 机制以文字的形式沉淀下来，主要是想分享给更多的人。若是这些文字对某些人、某些团队有帮助，那对于我来讲是一件使人欣慰的事情。若是能接收到关于优化现有机制的指点，也会是一件使人开心和感激的事情。

此外，还想表达的一点是：丁香医生前端团队是一个很是在乎每个团队成员成长的团队。

我猜，你可能猜到接下来我要说什么了。

是的，随着丁香医生业务的发展，咱们须要优秀的前端同窗加入咱们，一块儿茁壮肆意成长。更多关于团队的介绍，能够参考请问丁香医生前端团队怎么样？

招聘 JD

高级/资深前端工程师

职位描述

• 负责丁香医生旗下产品的前端开发工做（网站，Web App，Hybrid App，微信小程序，管理后台，Node.js 中间层）；
• 依据产品的需求，优质高效的完成前端项目的开发和维护；
• 对产品的前端性能进行优化，确保产品具备优质的用户体验；
• 参与丁香园前端团队的基础平台建设；

任职条件

• 3 年以上前端工做经验；
• 熟练使用 HTML（HTML5）、CSS（CSS3）和 JavaScript（ES6/ES7）；
• 熟悉网络协议（HTTP/SSL）；
• 熟练使用 Webpack 或者 rollupjs；
• 至少熟练使用一种 CSS 预处理器（如：Less、Sass、Stylus）；
• 至少熟练使用 Vue.js、React.js、AngularJS 三种框架中的一种；
• 对前端开发规范、工程化、组件化、测试有必定的认识和实践；
• 理解并熟练使用面向对象编程思想，注重设计模式、模块化开发在实际项目中的应用；
• 较强的责任心，良好的沟通能力和文档编写能力；

优先条件

• 在简历里写明 Github 帐号或我的博客地址；
• 独立开发过或者参与过优质的开源项目；
• 有实际 Hybrid App 项目开发经验；
• 有实际的微信小程序项目开发经验；
• 有高负载场景下 Node.js 应用开发和运维经验；
• 熟练使用 TypeScript；
• 熟悉使用一门非前端的编程语言（如：Java、PHP、Python、Go）；

前端实习生（全职）

职位描述

• 负责丁香医生旗下产品的前端开发工做（网站，Web App，Hybrid App，微信小程序，管理后台，Node.js 中间层）；
• 依据产品的需求，优质高效的完成前端项目的开发和维护；
• 对产品的前端性能进行优化，确保产品具备优质的用户体验；
• 参与丁香园前端团队的基础平台建设；

任职条件

• 对编程技术有热情，指望本身在技术上有快速成长；
• 毕业前可以全职实习至少 6 个月；
• 熟练使用 HTML（HTML5）、CSS（CSS3）和 JavaScript（ES6/ES7）；
• 熟悉网络协议（HTTP/SSL）；
• 理解并熟练使用面向对象编程思想，注重设计模式、模块化开发在实际项目中的应用；
• 较强的责任心，良好的沟通能力和文档编写能力；

优先条件

• 在简历里写明 Github 帐号或我的博客地址；
• 独立开发过或者参与过优质的开源项目；
• 熟练使用 Vue.js、React.js、AngularJS 三种框架中的一种；
• 有实际 Hybrid App 项目开发经验；
• 有高负载场景下 Node.js 应用开发和运维经验；
• 熟练使用 TypeScript；
• 熟练使用一种 CSS 预处理器（如：Less、Sass、Stylus）；
• 熟悉使用一门非前端的编程语言（如：Java、PHP、Python、Go）；

既然已经看到这里了，不如发一封邮件咱们聊一下吧：lizy@dxy.cn。

本文做者：丁香园前端工程师 @志遥