| 2022-04-12
什么是 Mob CR?
Mob: 扎堆。
Mob CR 是 Mob Code Review 的缩写。
中文的意思是:扎堆代码检视。
它的名字源于:Mob Programming,介绍参见:
【视频简介(短,英)】
【PPT简介(英)】https://docs.qq.com/pdf/DYk9xZkpwak94SGxH
Mob CR的形式是什么样的?
1. 团队找出一段需要被检查的代码片段。
一般来说,400 ~ 600 行的代码量。如果对代码比较熟悉,最多可以到 1500 行。
2. 独立代码检视。
参与 Mob CR 的所有人事先自行对这段代码进行单独代码检视,标记找到的问题。
这一步与正常的 CR 一样。
3. 约定一个时间段,并事先推选一名老司机,主导这次 Mob CR。
4. 正式 Mob CR
在老司机的带领下,对多人的检视结果进行做个检视,逐条梳理每一条评论。
直到所有人的评论都被检视并讨论过。
对有分歧之处,进行讨论,并得出最终结论。
得出最终结论是重中之重。
5. 代码清扫
这是本次 Mob CR 的最后一步,即:
指定一人将整个代码片段按结论对这段代码进行清扫,并合入生产代码库。
Mob CR 的目标
通过集体实战,快速让团队技术负责人
- 掌握当前所有人对代码质量的认知水平。
- 教育新成员或后进者。
- 快速建立(或重建)团队对代码工程设计质量的标准与要求。
什么样的团队应该做 Mob CR?
原则上来说,只要没有对代码的好与坏没有形成统一且良好的认识,没有形成工程纪律的团队都应该通过 Mob CR 的方式,将活生生的自己的代码示例当做靶子,对齐团队对代码质量的统一认识。
通常来说,仅仅要求团队成员阅读规范文档是无法形成这种统一认识的。
这些团队的特征包括:
- 新成员比较多的团队
- 新组建的团队
- 团队长时间没有做过代码Review
- Code Review 中发现较多问题的团队
一次 Code Review需要注重哪些内容
这次 Mob CR 发生在 2021 年 1 月 9 日晚上,从晚上 7 点到 11 点。
每次 CR 需要注重哪几个内容?
至少可以分四个递进层次。分别是:
- 自动化扫描工具(机械式)
- 代码基本规范(人工)
- 现有的实现与设计(人工)
- 我是否有个更好的实现方法(人工)
第一层:自动化扫描工具拦截
就是当代码的作者提交给代码检视者进行检视之前,由事先定义好的自动化扫描工具阻挡一波。
成功通过自动化扫描工具的检查之后,就可以真正投入人力了。
第二层:code-standard-smell 或明显的代码缺陷
在此步骤中,
若本次的代码变更中有太多基本的规范性问题,可能直接打回让作者修改,而不必
例如:提交注释是否清晰达意,是否与所提交的代码内容相吻合,
第三层:代码设计
从代码维护者的角度阅读代码,发现代码可能存在的问题或安全风险,或者更好的优化点,并指出来。
能发现代码中的 Bug 当然好,
但是,并不应以每次都找到现有代码中的 Bug 为代码检视的主要目标。
尤其是在做日常 CR(即每次提交代码都做CR时),
「能够发现可能的 Bug 或安全风险等」属于 Nice to have
。
更多应该强调的是:
这次代码变更是否让代码库的整体质量变得比原来更好,更易读懂,以后再来修改这段代码时,不会因为代码太差而改不动。最不济,也至少要不能变得更差。
第四层:是否有更好的实现方法
代码检视者针对这段变更,是否能想到比目前的实现方案更好的方法。可以提出来,与这段代码变更的作者进行讨论。
Mob CR 的一个示例
下面是一次 Mob CR 的简单记录。
这次 CR 的代码规模
6个代码文件,1529行代码,在CR之前一共有130个有效 Review Comments。
第四步「正式 Mob CR」的过程一共历时 4 个小时。
发现的规范性问题如下
本图并非全部,仅为了展示。
以下与代码上下文有关,仅为了示例。
发现的主要设计问题如下:
老司机指出,对于其中一个配置文件,如果用 protobuf 把配置定义清楚,直接交给不同的语言去 unserialize, 要比弄个没有 schema 的 yaml 格式文件要好得多得多。而且,这样一来,各个语言也都能保证一致性。
同时,老司机还给出 API 的设计原则,应该是「面向资源的 API 设计」。
同时给出了谷歌用的 API 设计指南,链接在这里。
本设计指南的目标是帮助开发者设计简单、一致且易用的 API。