代码审查过程

　　 英文链接： Code Review Processes

　　对我而言，把代码产品化而没有合适的审查流程，就像是一场抽抽乐游戏。代码当然也有可能会挺好，不过总还是有一定概率某人的哪块积木没抽好，然后一切就轰然崩塌。无论是采用持续集成服务、结对审查、QA审查，还是所有这些方案的组合，都可以大大降低引入风险的概率。

　　 编程团队规模已经超过了你的掌控能力

　　我在 Think Through Math（下文简称 TTM）的工作有18个开发者组成的工程团队，其中有两位是QA专家。这算是有挺多人（相对而言）在同时编码了。我们的架构中有好几种不同的服务，所以不是每个人都能对代码的每个角落了如指掌。我们中有些人偏重于前端的工作，有些人侧重于数据仓库和报表，还有些人则在后端折腾Ruby代码。我们都会经常重新搭配分组以相互传播知识，不过始终还是有相当多的人在为不同的项目工作，而没有一个人能把整个系统吃透。

　　我知道在这种事上我们并不孤单——有其他开发者也处于差不多的情况。一些人只是按部就班开发着应用，一些人则尝试把庞大的Rails应用拆分成小粒度的服务，还有一些人在处理遗留代码拯救项目。不管是什么类型的项目，都可以从某种恰当的代码审查流程获得很大收益。对按部就班的开发项目而言，可以确保做修改，添加新特性时一切正常；对大型系统拆分而言，是可以在代码质量改进并正确分解的时候确保每个新服务都仍保持功能；而对于拯救项目，则是可以确保代码适配新标准时，所需的功能都正常。更棒的是，流程合理的话这些都可以自动完成。

　　 TTM当前审查流程

　　在我们TTM的整个审查流程的迭代中，我不记得有哪次是不好使的，不过总有一些比其他更有效。最新的一代似乎是目前为止最好的（如人所愿），不过仍然存在改进空间。下一篇帖子里我会很乐意解释我们针对审查流程达成共识和改进的新方法，不过这不在本帖讨论范围。下面是我们当前审查流程执行的要点：

　　 代码审查流程

　　首先，开发者（通常是一组结对），完成了bug修改，新特性或重构，把代码提交给Github。如果他们只是想要CI服务器在他们提交的代码分支运行测试套件，他们会打上我们的WIP（半成品）标签，这样其他开发者和机器人就知道这次什么都不用做。这时（还有每次代码提交PR时），我们的Hound服务器会运行来检查我们的代码风格，以确保提交的代码的风格满足我们所选用的代码风格。

　　代码一旦就绪，开发者就为PR打上“需要代码审查”标签（Needs Code Review label）。他（她）还会附上完备的清单形式的关于QA该如何检测PR的指南。在预设的时间以后，我们的能干的聊天机器人，mathbot会来检查带着“需要代码审查”标签”的PR（Pull Request），给它们分配一两名其他的开发人员来审查代码。若是某个代码审查者觉得审查自己不熟悉领域的代码不太舒服，他们只需在聊天室里请求熟悉相关代码的人来替换。

　　我们通常会要求开发者仔细检视代码，而不是简单地做个人肉 lint 工具或者编译器。从我们的流程定义页面照搬的对审查风格的要求是：

• 保持好奇，审查PR的作者为何如此实现；
• 查找逻辑上的瑕疵，写反了的布尔判断，潜在的对象的错误选择，类型不匹配；
• 寻求代码命名和动机的清晰；
• 寻找可以优化的地方。太细的优化并不常需要，主要是能够避免的东西如：
• 循环内部的偶发DB调用
• 循环内部可以避免的繁重工作
• 频繁使用和代价高昂的操作，却没有使用缓存或记录的机制

　　如果审查者或其中一人认为代码可以调整，他们就会在Github的PR添加简短评论，开发者就能获得一个很清楚的标注，说哪些地方有问题。之后审查者移除“需要代码审查”的标签并替换成“需要修改”的标签，提醒开发者关注评论，并修改实现，或者申辩这样编写代码的原因。

　　有一件重要的事情需要注意，这里所有的评论都是以尊重和积极的态度完成的。我们不是为了对别人评头论足。我们是为了帮助彼此尽可能写出最好的代码，让整个系统更完善。自我提升是我们整个团队共有的目标，作为一个学习型的团队，我们总是在技术和实践上互相帮助。

　　要求的修改完成后，或者代码被证实合理后，开发者移除“需要修改”标签，重新标记为“需要代码审查”，这样审查者就可以继续干活儿了。一旦他们都签发（sign off）了本次改动，“需要代码审查”标签就翻成了“完成代码审查”，也就是不需要再做代码审查了。如果不需要用户体验评估，那么”需要QA”标签就会打上。

　　 用户体验评估

　　我们的产品有广大受众——学生、学校的教职员、管理员等。所以我们非常关注我们版本的变化和用户界面/体验的变化。据此，一旦我们的系统前端做出了任何可能使站点上某项外观变化的改动，我们都需要为PR加上“需要UX”标签，以使UX小组的某人可以审查我们的变化。因为UX要改变通常也影响到代码，所以这个步骤一般和代码审查流程并行完成。如果需要再修改，审查者移除标签并设置为”需要修改“，如同之前代码的流程。一旦都好了，他们会移除”需要UX“标签并带上”需要QA“标签。

　　 QA审查

　　QA小组很小，不过职责重大，要确保我们的代码满足业务需求，别上来就崩溃了。我们使用项目管理工具把PR链接到故事，做QA工作的人进行审查时，就可以快速地参考相应的验收标准，以及目标和偏差。他们使用开发者提供的关于测试内容的说明，动用他们自己的能力尝试去击垮系统。如果有什么地方不对了或者没有满足验收标准，他们还是移除所有标签，加上”需要修改“标签，这样在开发者实施修改之后几乎总是会强制带来新一轮的代码审查（通常只是修改部分）。当所有事情都做完，运行良好，代码会得到一个”Passed QA“标签，然后会被合并到我们的发布候选分支。到此代码会最终提交给项目master，并在QA和产品团队决定发行版中应该包含的特性和bug修复后提交产品化。

　　如果这些标签的玩意让你迷惑，请看下面的流程图。

　　 为什么？

　　流程看起来很多，其实在真实世界里使用起来真的算极其简单了。如果是一次小审查，一个需要尽快完成的特性，整个工作流甚至可以在十分钟内完成。不过通常来说时间会长些，并没有那么急。一切都自然地协作，这套流程确保了我们有三类不同的人群，从不同视角看待我们的代码，确保它有很高的质量并完成应有的功能。这有助于保持我们的代码库和测试的整洁、可读，以及自我文档化。

　　好处可不只是你有了一个流程。我们发布更少的bug，我们对进入代码库的代码满怀喜悦，而且还有人帮助我们每天提高。我无法强调有多少次第二双眼睛阻止了我代码中愚蠢的错误或性能下降。还有QA，在我没有检查的时候，他们救了我无数次。我从未觉得有评论或者批评是带着怨恨或者审判心态的，它们只是坦率的呵护，为了保持代码整洁，为了帮助我修正错误，清理代码，或者学习新的东西。

　　 总结

　　在你实际工作中做一些审查的流程吧，或者花点时间去分析，回顾并改进你既有的策略。迭代你的流程，拥抱潜在的变化，因为下一个主意就可能更好地改善结果。软件和真人同时审查你的代码，能避免你把愚蠢的错误发布到产品中去，能让你的代码仓库中堆砌的不再是无法管理的代码。