工程质量-05-第一阶段：围绕代码审查建立基本流程

[  code-quality  ] 25 Jan 2022 • 3 min read

这一个阶段主要做几件事件

• 将代码分支规范化，确认各版本主分支，以便进行精细化维护
• 引入简单但是有效的一些静态代码缺陷扫描工具
• 建立起代码审查机制
• 可选择的加强方案审查与风格检查的工具

第一个阶段的目标是先投入一些简单，并且不会对当前工作造成中断的流程，同时见效很快的工作。这个阶段中，最重要的工作是建立起代码审查机制。

在这个阶段，我们希望能够将我们的错误检出率提高的目标表如下

Removal Step	Lowest Rate	Modal Rate	Highest Rate
非正式的设计审查（Informal design reviews）	25%	35%	40%
非正式的代码审查（Informal code reviews）	20%	25%	35%
静态代码分析（Static code analysis）	5%	10%	15%
集成测试（Integration test）	25%	35%	40%
系统测试（System test）	25%	40%	55%
缺陷排除预期累计效率值	67.93%	82.88%	91.04%

整个流程分为三个小阶段

• 第一个阶段主要建立起一个基本的CI流程，将代码分支管理、静态检查待基础功能做好。
• 第二个阶段引入代码审查制度
• 第三个阶段，如果能够的话，可以规范代码风格和加强方案审查

5.1. 建立静态检查和CI自动化流程

在首先第一个小阶段，我们希望你能够快速的建立起一套自动化的流程。将工程里面的一些能够使用工具的地方自动化，这样能够马上形成一个小闭环，形成非常正向的反馈。这个工作主要包括

1. 确定主杆和迭代模式

基于Git的版本开发一般有四种分支模式TBD、Git-Flow、GitHub-Flow、GitLab-Flow[7]。如果在这一块还比较混乱的话，我建议目前先可以确定一下开发分支的形式。但是无论何种形式，对于后面的有几个要求

如果目前评估代码的质量或者整体的工程质量比较差的情况下，不建议采用TBD单分支这样的开发模式，单分支开发模式要求新的功能直接加入到主分支里面去了，如果质量比较差，特别容易引起主线上的问题。

1. 需要确定一个主杆分支，并且保证未来的新功能开发、版本发布、打TAG都是是基于这个主杆分支之后的。后面的很多流程都在这个主杆上面进行，保证合并到这个分支的代码符合要求就可以了。不需要所有的分支一下就添加所有的流程。随着时间推移这个分支上进行演变，分裂出其它的分支，自然整个项目就规范起来了。
2. 后面涉及到很多对代码的改动，无论采取什么样的模式，希望的是快速迭代，快速合并，不要等到积累了一大堆的改动之后，再合并到主干上，那样，可能会不敢使用最新的代码。
3. 无论是当前正在开发新的功能，还是目前处于修整期。修改一点，合并一点。这样对整体的可靠性会提高很多。

2. 在CI上面将工程自动化编译起来

在CI上面，通过Git runner之类的机制将当前的工程编译成功，这是添加的第一道编译检查。（有一些脚本语言是没有编译过程的，在这个阶段可以不做这个。）

编译的工作强烈建议放到Docker里面来完成。现在的Docker容器技术已经很先进了，将编译的环境放到一个Docker镜像里面，一方面可以将这个环境移动到各种环境里面去，另一方面编译中需要安装的各种环境依赖也不会污染了服务器本身的环境。包括嵌入式项目，也可以通过放到Docker里面进行交叉编译。

让后面每一次提交的代码都能够自动的编译一次，这是加强代码的第一道关，也是最简单的一道。没有通过编译的分支不能够合并到主干上面。

3. 添加自动化代码静态检查

在完成编译之后。我们已经建立起了最简单的闭环，接下来，可以为自己的语言添加一些静态代码缺陷扫描工具。目前市面上根据不同的语言都有很多的静态缺陷扫描工具来进行检查，比如这里有一张表，你可以查询一下自己的语言是否有更合适的静态检查工具https://en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis。对于这样的工具有几个建议

1. 将静态检查使用Docker来运行，不要在物理机上跑，使用Docker利于安装和迁移。
2. 一般的静态检查可能会有很多配置，比如忽略某一些检查之类的。在这一个阶段，你不需要强制引入过强的规则，以避免对当前代码进行大的修改。这种大的修改等到后面代码审查机制运行一段时间之后再来进行考虑。
3. 如果自身的代码本身并没有严格遵守某种代码风格，在这个阶段不建议引入代码风格检查工具。代码风格检查有可能会对当前的工程代码造成过大的修改。初期不利于这么种的操作，等后面代码审查机制运行了一段时间。整个工程新版本开发的时候再考虑这样重量级的改动。

强调一下，如果这个阶段静态检查或者代码风格检查发现的问题过多。你需要做的是在合理的范围内，尽量的将某一些检测规则跳过去。让这个流程跑起来，初期形式大于意义。如果屏蔽不了，你可以将这个流程在CI配置为一个不会中断的流程，即便过不了，也无所谓。

有了这个检查之后，后面分两个阶段

1. 确保每一次新添加的代码或者修改代码的相关文件都需要过这个静态代码检查流程。没有修改过的代码，就不要去管它。
2. 迭代几次版本之后，整个项目中有80%左右的文件都已经通过了这些检查，那么就可以考虑集中一次性将剩下的问题都解决。从而将这个静态检查由可选，变成强制。

通过上面的流程之后，目前您现在的应该如下所示

5.2. 引入代码审查流程

在第二个小阶段，我们会引入代码审查机制。这是一个比较大的流程，在引入之前，我们会简单阐述一下，代码审查对我们意味着什么，会带来什么样的好处。然后会具体简介一些代码审查的形式以及关键点。

5.2.1 为什么要做代码审查

代码审查首要的好处是通过人工来确认代码的正确性，能够发现一些BUG，是不是与设计相符合。除了这个显而易见的好处之外，代码审查还有其它很隐形的好处。如下所示

• 保证代码的正确性：有另一双眼睛能够看到当前代码的功能，这方面非常容易发现代码中的错误。每一个人的思路都不一样，当一个人按照他的想法实现了某一个逻辑功能，另一个人在理解他的代码的时候关注的方面往往是不一样的。正如写作的时候，自己写的东西很难发现其中的错别字，其他人读的时候就特别容易发现。除此之外，正确性还包括代码的实现是否符合设计，是否完整的实现了需求。
• 确保其他工程师能够理解代码的更改：一份代码，正常情况下会被写一次，然后被看很多次，所以能够确保代码能够被理解是非常重要的。代码审查是一次非常好的可读性测试，以验证当前的代码是否能够被其他理解，包括未来的自己。这会促进编写者在写代码的时候考虑可读性，使用简单的表达，增加必要的注释。
• 保持整体工程代码设计和实现的一致性：每一个人都会形成自己的编程风格，但是当我们处于同一个团队的时候，保持统一的风格非常重要，这个风格不仅仅包括代码风格还包括设计风格、处理问题的一般套路等等。通过代码审查，我们能够很好的保持这种风格。
• 促进知识共享：代码审查有助于团队之间的知识共享。新手可以通过代码审查的反馈以提升自己的技能，快速熟悉当前项目常用的一些约定，这比费心维护文档好多了。新加入团队的成员也可以通过代码审查的更快的熟悉整个工程。一些好的编程实践也可以在代码审查过程中被其他人所知。

我觉得知识共享，是代码审查最重要的好处之一，正如前面立的原则一样，研发是以人为本，写代码是一件很关注细节的工作。要规范一个团队的细节的一个好的办法是在代码审查中不断的总结一些最佳实践，不断的分享一些好的代码处理细节。日积月累之下，就可以慢慢形成团队的统一风格和规范，在代码审查的开展过程中也可以带着新人传承。这是整个团队工作过程中必不可少的一个环节。

5.2.2. 代码审查的演变史

IEEE一共定义了五类审查机制[9]，随着软件工程的发展，一些耗费人力比较重的审查机制逐渐被一些轻量级的审查机制所替代[10]。从历史发展的角度来看，应用得比较成功的主要有三类审查机制[8]:

• 传统的正式代码审查法（Software Inspection 范根检查法）:主要包括，Planning，Overview，Preparation，Inspection Meeting，Rework和Follow-up几个步骤，需要专门的人扮演不同的角色来推动流程进行。全文检索引擎Lucene，就是使用的这样的代码审查机制，根据资料记载[11]，一次审查流程大概平均需要10天的时间。现在这种方法由于比较费时间和人力，因此使用的公司已经非常少了。
• 开源软件的邮件代码审查(Open Source Software email-based peer review)： 主要是通过研发都打一个patch，然后通过邮件发送给相关的审查者进行审查，经过几轮审查之后，符合标准，就将这个patch合并到主线上。一些比较老的开源软件就是基于这样的代码审查方式，例如Linux operating system, the FreeBSD operating system, KDE desktop environment, and Gnome desktop environment。
• 轻量级工具支撑型代码审查(lightweight tool supported review)：基本流程与开源软件的邮件代码审查很像。不过核心区别是代码审查是有一个工具来统一管理的。在代码审查之前会经过一系列的静态扫描，之后再进行人工的代码审查。审查通过之后，通过工具自动合并到主干上面。目前的微软、Google等大部分企业都是采用的这种代码审查形式。

现代的代码审查基于上都属于轻易级代码审查，主要有三个特征：审查非常频繁，使用工具辅助，以修改的代码为基准进行审查。一般叫它Regular change-based code review (Walk-throughs)[10]。

5.2.3 怎么做代码审查

在[5]中，描述了Google认为的代码审查应该审查的内容，很具有参考意义。代码审查一共审查三个方面，通过三个角色的审查

• 一位工程师对代码的正确性和可理解性进行检查，确认代码是合适的，并且设计和作者想要完成的功能是完善的。
• 当前分支或者模块的负责人对代码进行审查，以确保代码的修改是符合这个模块的迭代意图。
• 一个人专注于代码的可读性进行审查，确保代码符合了相关的技术规范（代码风格、特殊约定）等等。

当然在实际的过程中，并不是真的需要三个不同的工程师来分别审查这方面，虽然多个工程师审查效果是会好一些。但只要保证审查完了这三个方面，一位工程师也可以。

代码审查一般是在提交代码，并且通过了基本的静态检查和自动化编译，单元测试的流程之后，在合并主分支之前进行代码审查，通过人工提交一个Merge Request通知第三方人进行审查。

5.2.4 代码审查的最佳实践

[8]统计了大量比较著名的项目的代码审查数据，主要涵盖包括：Android、Chrome、Bing、Office、SQL，这里面有互联网项目，也有定期发布的操作系统数据库项目，总结了在代码审查中非常有意义的最佳实践

1. 现代的代码审查的形式非常轻量级，同时流程很灵活

一般包括几个步骤

1. 作者提交一个请求进行代码审查
2. 审查根据作者的代码进行审查，并且提出问题，解决问题
3. 通过几轮审查之后，审查者这次提交打上同意的标签。这次代码就可以合并到主线上面了。当然如果审查没有通过，则可能拒绝合并。

2. 代码审查发生的时间越早越好，越快越好，频率越高越好，平均在一天时间内处理完

工程开发有一部分功能之后，就应该提交代码进行代码审查。而不要等到写得差不多了，才进行代码审查。每一次代码审查的首次响应时间为一天左右，最好不要超过一天。如下图所示，这几个项目最佳的首次响应时间都是一天左右

代码审查非常频繁，如下图所示，有一些项目每一个月会高达上千次代码审查。当然不同的项目由于发布版本的周期不一样，代码量不一样，所以有时候会少一些，有时候会多一些。但是这些数据可以对应到自己的项目上，为自己代码审查频率设置一条线：

3. 每一次代码审查的量越小越好

上图可以看到，不同的工程，每一次审查的代码都比较少，大部分集中在150行以下。每一次代码审查的量越少，审查者能够更加集中，审查的效果最好。

4. 每一次参与审查的人2个最好

代码审查的人数越多，并不能够显著的提高代码审查的效果。上图的统计发现，一般的代码审查两个人的效果是最好的，找两个最熟悉这一块的人进行代码审查就可以了。对于这一点[12]对几百个项目进行统计，发现平均而言1.56个人进行代码审查是最合适的。

5. 代码审查从发现问题变成讨论解决方案的流程了

每一次代码审查平均会获得3到4个评审意见。这些大部分并不是在找问题，而是在讨论解决方案。

5.2.5 引入代码审查机制的一些建议

在代码审查的过程中，做好几件事情

• 第一阶段：形式大于意义
  • 主要目标：让所有的人接受代码审查这件事情，同时接受如何更平和接受别人的建议。
  • 开展形式：
    • 只在合并到主分支上面进行代码审查
    • 每一次新增加功能、修改BUG的时候进行代码审查，线上进行就可以了。
    • 每一周定期举行集体代码审查（两个小时左右），审查那些系统中最疑难的部分。审查过程中同步建设审查文化。
  • 结束的标志：正常情况下，一个月左右，大家就都能够接受这种形式了。
• 第二阶段：聚焦于问题，建立规则
  • 主要目标： 根据前面一段时间的审查，团队积累了很多最佳实践，同时也积累了大量的疑问。这个阶段主要目标是需要建立起一个明显的代码审查检查表。比如叫《代码审查规范》。至少要定义清楚，重点审查哪些方面、每一次提交的代码量多少为益、每一次响应的时间多久合适、每一次的代码需要有哪人来审查。
    • 你可以根据本章提到的Google的最佳实践，规定代码审查的时候需要审查三个方面，需要哪些人来进行审查。
    • 你可以参考一下别人的团队代码审查规则来加入到自己的代码审查规范里面。
    • 你可以根据自己的行业和项目特点，制定更适合自己的一些规则。
  • 开展形式：
    • 每一次合并到主杆的代码都需要审查。
    • 每一周进行一次集体代码审查，依然可以审查重点代码。但是更重要的是总结一下代码审查的经验，形成适合《代码审查规范》
  • 结束的标志：《代码审查规范》建立一个初步的，能够运行起来，覆盖代码审查整个周期的主要注意事项就可以了。《代码审查规范》后面还可以慢慢迭代改进，不求一次性就弄好。这个阶段可能会持续三个月，完整迭代一个版本左右的时间。
• 第三阶段：稳定迭代、常态化
  • 主要目标：强调质量，不断的优化《代码审查规范》。你只要对每一个新出现的BUG，一直不断问自己一个问题，为什么通过了代码审查，这个问题还是给漏了？
    • 如果是代码审查应该看到，但是没有看到的，分析其原因。适当的时候加入到代码审查规范里面去。
    • 如果这并不是代码审查能够注意到的，那么可以想一想在其它什么流程上可以加强一下，规避这个问题。
  • 开展形式：
    • 每一次主干合并进行代码审查
    • 每一周进行一次集体的代码审查。主要讨论新出现的BUG为什么漏，讨论方案，讨论是否更新《代码审查规范》
  • 结束的标志：没有结束

5.2.6. 引入代码审查机制成败的关键

• 一定要建立明确的规范：凡事预则立，不预则废。代码审查参与的是人，人都有发挥不稳定，疏忽的时候。如果没有一个明确的检查表约束一下，强调每一次代码审查一定要审查哪些内容。人就特别容易给漏了，代码审查就容易流于形式。
• 代码审查不是万能的，不要搞复杂了：前面的DRE表里面，代码审查的效率就只是25% - 35%左右。代码审查找BUG的效率并不高，它还有共享知识，共同学习，讨论方案的目标，这个有时候比找BUG更重要。
  • 代码审查它并不能够解决所有的问题。如果太过于看重代码审查的效果，就特别容易把《代码审查规范》搞得特别细，特别复杂。这就会让代码审查成为一个巨大的负担，从而降低代码审查的效率。
  • 我觉得一个团队的《代码审查规范》不要超过一页纸，越少越好。如果有更新的规则，那就需要考虑一下是否需要将以前的一些规则合并删除掉。
  • 我们原则里面强调的是变主观为客观。代码审查即使添加上了各种规范，依然还是很主观的事情。代码审查达到一定的程度，就不要强求质量了，将这些精力放到一些自动化的流程上去，用机器自动化的方法来逐步替代人工的审查。比如代码风格审查，能够使用工具，就使用工具，不要让人来特别关注这一块。
• 初期可以上审查下，后期一定要建立平等审查的氛围：如果团队里面存在比较强的等级，为了更好的推进代码审查，在初期的时候可以让高级工程师审查初级工程师的工作，主管审查高级工程师的工作，主管的工作由某几个特定的人审查。但这只是初期的权益之计，最级当代码审查成为一种良好的氛围之后，还是需要大家平等来进行审查。不要浪费任何一个人的才能，这才是真正的团队合作。

5.3. 代码风格与方案审查

代码风格和方案审查属于如果可能，就尽量去做的范畴。在代码审查经过一段时间之后，可以根据情况决定是否来加入这些流程。

5.3.1. 关于代码风格

代码风格并不直接提高问题检出率，但是却对代码的可读性，可维护性非常重要。有很多团队，虽然声称有代码风格，但是实际上对风格的遵守并不严格。一个好的代码风格应该具有以下几个方面的特性

1. 有比较完善的规范性文档。
2. 有自动格式代码工具，最好能够嵌入么IDE里面去。大部分代码风格不应该是由人来记的，而是通过工具来自动化完成的，只要有一点可能性，都应该使用自动化工具来完成。
3. 有自动的代码风格检查工具。

这三部分都有之后，才能够算是一个完整的闭环。基于这个特性，我强烈建议代码的基础风格可以参考成熟团队的代码风格。比如Google的C/C++、Java、Python相关的风格，都有配套的自动化工具来格式化和检查代码是否满足标准。

一个工程如果一开始并没有完全遵守某一种代码风格，后面想来修改其实是很难的。通过风格扫描工具会暴露一大堆问题。如果集中力量一次性的将代码风格调整好，那么代码变化过大，不利于其它分支的合并。

引入代码风格扫描工具，可以分几种情况

1. 如果你的项目才刚刚开始，代码量很少，还没有发布过版本，那啥也不说了，直接在项目最开始就把代码风格相关的工具配置起。避免以后反复。
2. 如果你的项目其实很遵守某一个代码风格，工具扫描出来的问题并不多（可以有选择的屏蔽一些规则）。那么也可以考虑将代码风格一次性调整好。
   • 这个并不多，主要是指的已经确定这些风格调整不会引起主功能的变更。
   • 在一次提交测试之前调整就可以了。
3. 如果你的项目没有遵守某一个规则，或者通过工具扫描出来的问题特别多。那么我建议先不要调整，慢慢随着功能迭代调整
   • 首先对新增加的功能，修改相关的BUG，影响到的文件或者模块进行调整
   • 或者每隔一段时间就挑一个代码风格的问题调整一下
   • 随着迭代开发，慢慢改进，如果评估调整已经达到90%了，剩下的部分风格只需要简单调整就可以了，可以像第2条一样一次性的调整一下。随着提交测试，完成这个工作。

代码风格，代码工程规范化，其实可以考虑统一开发环境。现在的主流语言使用Docker + VSCode remote development可以搭建一个统一的远程开放环境。在Docker里面将代码风格检查工具、编译环境这些都统一配置好，这样很多类似风格和编译问题都会简化很多。

5.3.2 关于方案审查

值得一提的是方案审查。如果你的项目处于刚刚开始的阶段或者正好有方案审查，我建议您在方案审查上面尽量的下苦功夫，这个阶段的投入是有价值的。最佳实践可以看看第三阶段里面的内容。

但是事实上，我们大部分的团队维护的都是已有项目的代码，很少有团队正在从零开始。如果你的项目正在发版迭代，那么您的团队应该是有一个方案审查流程，无论这个流程怎么样，它一定存在。如果已经存在了，在第一个阶段不需要特别去优化它。所以方案审查在这个阶段并不是重点，保证当前水平就可以了，方案审查将会在最后一个阶段中认真描述。

引用

• [1] Demarco T, Lister T. 人件[M]. 3. 机械工业出版社, 2016 :22-23.
• [2] McConnell S. Code Complete[M]. 2nd ed.. Microsoft Press, 2004.
• [3] Ariel Assaraf. This is what your developers are doing 75% of the time, and this is the cost you pay[EB/OL]. 2015 https://coralogix.com/blog/this-is-what-your-developers-are-doing-75-of-the-time-and-this-is-the-cost-you-pay/.
• [4] Jones C, Bonsignour O. 软件质量经济学[M]. 1. 机械工业出版社, 2014.
• [5] Fournier C. Software Engineering at Google[M]. First Edition. United States of America:O’Reilly, 2020 :27-42.
• [6] Capers Jones. Software Defect Removal Efficiency[EB/OL]. 2011. https://www.ppi-int.com/wp-content/uploads/2021/01/Software-Defect-Removal-Efficiency.pdf.
• [7] 阿里技术. 如何选择 Git 分支模式？[EB/OL]. 2020-07-10[2022-01]. https://zhuanlan.zhihu.com/p/158463879.
• [8] P. C. Rigby and C. Bird, “Convergent contemporary software peer review practices,” in Proceedings of the 2013 9th Joint Meeting on Foundations of Software Engineering. ACM, 2013, pp. 202–212.
• [9] IEEE Standard for Software Reviews and Audits, IEEE Std. 1028-2008.
• [10] T. Baum, O. Liskin, K. Niklas and K. Schneider, “A Faceted Classification Scheme for Change-Based Industrial Code Review Processes,” 2016 IEEE International Conference on Software Quality, Reliability and Security (QRS), 2016, pp. 74-85, doi: 10.1109/QRS.2016.19.
• [11] A. Porter, H. Siy, A. Mockus, and L. Votta. Understanding the sources of variation in software inspections. ACM Transactions Software Engineering Methodology, 7(1):41–79, 1998.
• [12] Baum, Tobias & Leßmann, Hendrik & Schneider, Kurt. (2017). The Choice of Code Review Process: A Survey on the State of the Practice. 111-127. 10.1007/978-3-319-69926-4_9.
• [13] Fowler M & 熊杰. 重构-改善既有代码的设计[M]. 2. 人民邮电出版社, 2010.
• [14] Khorikov V. Unit Testing-Principles, Practices, and Patterns[M]. 1. Manning Publications, 2020.
• [15] Martin R C. Clean Code-A Handbook of Agile Software Craftsmanship[M]. 1. Prentice Hall, 2008.