如何完成一次高质量的Code Review?
(2020版)
Emac
2020/8
项目“死亡”三角
早餐店的故事
我们应该谨守“及早发现、及早解决”的准则，如此我们便能在生产流程中价值最低的阶段修正问题。因此，我们应该在蛋商送蛋之前让他们把坏的蛋挑出来，而不是让我们的顾客在餐桌上发现它们。
- 安迪·格鲁夫
CR 水平自测
CR 水平自测
- 搞形式主义，存粹是浪费时间
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
- CR 是 Leader 的事，跟我没关系
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
- CR 是 Leader 的事，跟我没关系
- 我只看指给我的 CR，其他 CR 跟我没关系
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
- CR 是 Leader 的事，跟我没关系
- 我只看指给我的 CR，其他 CR 跟我没关系
- 没有时间 CR，直接 Merge
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
- CR 是 Leader 的事，跟我没关系
- 我只看指给我的 CR，其他 CR 跟我没关系
- 没有时间 CR，直接 Merge
- CR 必须一行不落从头看到尾
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
- CR 是 Leader 的事，跟我没关系
- 我只看指给我的 CR，其他 CR 跟我没关系
- 没有时间 CR，直接 Merge
- CR 必须一行不落从头看到尾
- CR 必须一次完成
CR 水平自测
- 搞形式主义，存粹是浪费时间
- CR 是保证程序正确性的手段
- CR 是保证代码规范的手段
- CR 是 Leader 的事，跟我没关系
- 我只看指给我的 CR，其他 CR 跟我没关系
- 没有时间 CR，直接 Merge
- CR 必须一行不落从头看到尾
- CR 必须一次完成
我个人认为代码有这几种级别：1）可编译，2）可运行，3）可测试，4）可读，5）可维护，6）可重用。通过自动化测试的代码只能达到第3）级，而通过Code Review的代码少会在第4）级甚至更高。
- 陈皓
Subtitle
CR 的三重境界
认识CR：第 1 重境界
- 关注代码规范
- 避免重复代码
- 降低（圈）复杂度
- 关注性能问题
- 关注分布式事务
- 关注架构设计
认识CR：第 2 重境界
- 提升自我觉察能力
- 建立良好开发节奏
- 高频次的团队活动
认识CR：第 3 重境界
- 传帮带文化的重要组成
- 工程师文化的日常体现
CR “精彩”问题集锦
空白字符使用
命名不当
注解使用不当
位置不当
性能问题
架构规范
架构设计
整体Review
CR 最佳实践
- 依据个人偏好每天固定几个时间段专门用于 CR，我的习惯是出门前和下班前。CR 耗费的脑力丝毫不亚于编码，甚至更高，CR 过程中需要高度集中注意力。清醒的头脑和无干扰的环境，是提出高质量的评审意见的秘诀。
- 除了固定时间段，任务切换期间也是 CR 的好时机。
- 每次 CR 尽量控制在 15~30 分钟以内，超过 30 分钟应休息一会。
- 收到 MR 之后，先判断一下 MR 的性质，如果是 Bug Fix 类型的 MR，应尽快评审，如果是新功能MR，则可以等待下一个 CR 窗口。
- 从收到 MR 到 CR 结束，最长不要超过 1 个工作日。
- 开始 CR 之前先要搞清楚 MR 要解决的问题背景。
- CR 就像读书，先看目录（改动的文件列表），再精读重点章节（包含核心业务逻辑的代码），最后扫读剩余章节。
- 如果改动的文件数量较多，可以打开 IDE，切换到源分支，方便在 CR 过程中随时打开相关代码进行阅读。
- 评审核心代码时，如果发现严重问题，应立刻终止 CR，找 MR 提交者当面讨论。
- 如果 MR 提交者对评审意见提出异议，评审者应找提交者当面讨论，避免在评论区互踢皮球。
- 合并代码之前应确保所有评审意见都被妥善处理。
- 记得点赞。CR 不是只能提意见，看到优雅的代码不要吝啬你的表扬。