代码Review的正确打开姿势

[导读] 开发过程中，如何保证代码质量，code review是一个很好且必要的措施，本文来聊聊我对code review的一些体会。

代码为什么要review?

以为代码比较少bug，其实是这样的画风：

如果代码前期不提高质量，程序猿后期就可能会有这样酸爽的体验：

所以为神马要对项目代码做代码评审（code review）呢？一方面，直接原因是可以提高代码质量，将bug在开发过程中，测试前就给它揪出来；另一方面，团队协同作战，制定的代码规范怎么实施下去呢？code review也将是一个很好的措施，可以渐进的形成统一的编码风格，提升团队协同开发整体作战水平，并对于形成协同互动的团队文化产生积极促进作用。why？

• 激励认可，代码提交者提交一系列的代码交由审阅者审查（review），审阅者常由同事担任或者更资深的攻城狮担当，审查过程中不免有可圈可点之处被肯定；也有缺陷漏洞被识别，在肯定的方面必然会激励代码作者，被指出疏漏之处，必然能有机会堵住疏漏，降低后期查错的成本。对于大多数同学而言，获得前辈认可必然会被激励，获得指点，必然有助进益！

• 团队建设，知识技能的分享交流可以帮助团队：

• code review可以识别待改进项或者未完善或完成的任务，团队成员随后可以在已完成的工作基础上进行渐进迭代开发。
• 提交者可能会使用一种技术或算法，评审员可以从中学习获益。更为重要的是，代码审查有助于渐进提高整个团队的代码质量标准。因为审查过程中，就一个点会进行深度剖析，思想碰撞，所谓不辩不明。经过良好的长期的代码审查机制，团队的整体认知将会渐进提高！
• 积极良好的互动交流将有助于构建良好的团队文化

• 风格一致性：code review将有助于形成统一的代码风格，这是技术管理的一个重要维度。代码风格的一致性，也将有助于减少bug，提升协同开发能力，提升代码的可读性以及可维护性。为啥能提升协同开发能力？如果代码风格各异，比如一个项目多人并行开发，各是各的代码风格，你想象一下拼装集成起来是多么痛苦吧？一旦发现有bug，解bug又会是多么痛苦。

• 可读性识别，所谓当局者迷，旁观者清，代码片段的可读性对于作者本人来说很难判断，而对于没有完整上下文的阅读人员来说则很容易感知。可读性高的代码更容易重用，总体上来说bug会少些。

• 错误识别，不经意错误（例如，打字错误）或结构错误（例如，死代码，逻辑或算法错误，性能或架构问题）通常更容易被审阅代码人员从旁观的角度发现。即使是简短而非正式的代码审查，也会对代码质量和错误率产生较大影响。

• 合规性需求，比如在特定的行业，医疗、工业、汽车领域，代码审查是开发流程中强制要求的环节，比如功能安全认证，医疗器械认证法规体系，汽车电子等都明确要求需要进行代码审查并出具审查报告。

都Review些啥呢？

具体要Review哪些内容呢？这个问题貌似永远没有绝对正确的答案，每个开发团队都应该对自己的方法达成一致。所谓达成一致，团队成员需要一起制定代码规则以及Review的checklist（检查项列表），切忌由某一个人制定规则：

• 难免片面偏颇，即便是前辈大牛，也不能保证所有的理解都是完善正确的
• 需要团队align认同，否则难以实施，尤其团队成员水平相差不多，氛围文化还没有很好形成时，不免出现谁也不服谁的情形。
• 规则还可以协同渐进完善，但一定要是可实施的，而不是流于形式。

代码评审是需要无差别化，要一视同仁，即便是团队中最资深的人也并不意味着他的代码不需要评审。即使在极少数情况下代码是无瑕疵的，审查也提供了一个指导和协作的机会，并且加深团队对代码的理解。

具体实施可从两个方面着手：

• 团队协同定义代码规范，当然如果已有规范，需要对新人进行培训。代码规范没有绝对标准答案，只是为了建立一套成员都认可的代码风格以及习惯。
• 团队协同定义审查项列表，每个成员都可能是作者也可能是审查员，一方面在写代码时，就有了一把尺去度量，在审查时也有一个尺去检验。

对于审查项，比如可定义下面类似的规则：

• 检查通过引用传递的参数是否正确使用？
• 检查对于应实现返回的分支是否都实现了显式返回？
• 检查函数是否清晰易理解？
• 检查类型转换是否正确？
• ........

啥时候Review?

代码审查应该在自动检查（单元测试、代码风格、持续集成构建）成功完成之后进行，但是应该在代码合并到代码库的主干（或者叫mainline或者叫trunk）分支之前进行。

• 单元测试，相信做过的朋友应该会有体会，有效的单元测试将提前识别出bug
• 代码风格，主要是指代码的样式，比如缩进，花括号的写法，很多编辑器能帮助自动完成。比如IAR，比如Clang-Format，visual code的ident插件等。
• 持续集成，比如有很多公司会使用Jenkins搭建一个自动构建平台，借助这个平台可以实施自动集成编译、自动运行单元测试、自动运行代码静态检查等。

为什么要在这些自动检查成功之后才Review？因为这几个过程都有可能发现问题，并触发修改代码，所以Rview一个相对稳定的版本将会减少重复Review的工作量，节约时间。那么为什么要在合并到主干之前审查呢？这显然比较好理解，因为这样可以尽量将高质量稳定的版本或者功能合并进主干，可以保证主干的相对稳定以及质量。

该怎么Review呢?

发起Review的代码提交者，需要提前做好准备工作，从而提高效率，以减少浪费其他参与人员的时间：

• 明确范围：待审查变更应具有明确范，成体系的范围。例如，这笔更改实现新功能或修复的错误。较短的更改优先于较长的更改。对于一个涵盖多项需求用例的更改，比较好的做法是拆分成多次review，或者组织review时尽量条理清晰，做些分门别类的前期准备，以方便审查，提高效率。
• 仅提交完整且经过自我审查（比如利用版本管理工具的diff进行）和经过单元测试的代码。为了节省审阅者的时间，比较好的做法是在发送审阅者之前测试提交的更改（比如前面谈到的完成静态代码检查、单元测试、代码风格），并确保它们通过本地和持续集成服务器上的所有内部版本以及所有测试和代码质量检查。
• 代码样式提前整理好，人工审查需要消耗其他人的时间，代价可谓昂贵，所以应聚焦到程序逻辑上，而非样式，语法或格式的争辩上。更好的办法是采用工具自动完成，比如Clang-Format等自动工具来解决这些问题。

具体Review时，有哪些值得推荐的做法呢？

• 要针对代码不要针对作者，忌攻击作者，否则影响士气，不建议管理者将代码审查缺陷当作绩效考核指标。
• 要开放不要保守，没有人写出绝对完美的代码，助人亦是助己，建立良好的代码审查文化，以积极看待发现的缺陷。
• 要识别缺陷不要修复缺陷
• 要高效不要拖沓，不宜超过2小时
• 要合理节奏不要太快或太慢
• 要聚焦逻辑而非形式，如前面所说，不要浪费时间在语法、格式等主题
• 要追溯跟踪、落地实操不要形式套路，否则不如不做。对于识别出的问题，应逐一反馈关闭。
• 建议采用辅助工具进行审查
• 审查代码前，作者应准备必要的注释或者代码解释，以提高审查效率。
• 一次查看代码尽量控制在200-400行，太多则不易识别出问题
• 团队协同制定审查清单，并一致认同

总结一下

良好的代码审查机制，将有助于提升代码质量、减少产品缺陷、降低开发成本，同时也有助于持续提升团队凝聚力、建立更好的团队协作文化、提升整体的作战能力。而形式化的代码审查，则将浪费成本、影响士气。至后台发送CR，领取checklist