1. 发起CR
1.1 发起前准备
总结一个针对自身定制的checklist
是否有可复用的代码可以进行抽象
是否有现成的工具方法可以使用
是否移除了调试代码
每一个commit的log是否描述清晰
这个Merge Request是否可以再拆分更小
代码是否安全?是否易于维护?是否健壮?
把自己当作Reviewer来对自己的代码进行CR
预估代码可能出问题的地方
进行充分自测,保证代码可运行
不要指望别人帮你找出问题
1.2 利用工具进行检查
Code Review前置检查 通过严格限制Merge Request的条件,保证要Code Review的代码是有一定质量的。避免无效的Code Review,节省时间。
1.单元测试检查
2.新增单元测试检查
3.方法行数过多
4.圈复杂度过高
5.代码规范检查
6.客户端: 提交了较大的资源影响了包体积
1.3 简洁清晰的变更描述
1.3.1 减小MR的规模
比如, 有大量文件的提交, 大量代码的改动
1.3.1.1 什么算合理的规模
完整的单一功能变更
Feature:拆分
易于理解和阅读
变更行数尽量少于200行
尽量能再30分钟内完成审查
1.3.1.2 大规模提交应该怎么处理
选择关键Commit进行Review
在核心逻辑建议@reviewer作为审查重点
1.3.2 详细的commit描述
1.3.2.1 应该写什么?
摘要:概括变更内容点;
[x模块]新增xx功能
背景:此次变更的背景说明;
新功能需求,要求xx,附上需求链接
,说明:补充说明原因和备注甚至可能产生的影响;
由于旧数据不支持xx,新增xx处理模块
1.4 合适的提交对象与时机
能发现问题的人: 协作组员、导师、owner、资深的
提前沟通 避免节奏不一致,导致CR流程受阻
一次CR其实是开启的一次““对话'
应该期待评论和反馈,并及时进行回复
做好心理准备自己的代码可能会有缺陷
CR的目的之一就是发现问题,所以不要有抵触
2. 代码审查
2.1 基本审查
“代码是写给人看的,不是写给机器看的,只是顺便计算机可以执行而已。 —– 《计算机程序的构造和解释》
2.1.1 关于代码规范
舍弃个人偏好
要经过团队内的商讨
我们编写的代码不是我们自己的
代码必须符合制定出来的规范
2.2 代码审查看什么?
编码规范
潜在的BUG
文档和注释
重复代码
认知复杂度和圈复杂度
测试覆盖率
设计与架构
2.2基于团队背景下的审查
业务背景下的审查内容
项目规范
跟项目直接相关
U1使用规范
工具库使用规范
日夜间开发规范
踩坑总结
容易犯的错误总结
避免二次重犯
单测规范
单测的代码规范
如何写有效的单测
业务逻辑
具体业务逻辑
指定熟悉模块的人
结合需求做CR
2.3 高效沟通 & 冲突解决
2.3.1 高效沟通原则
不要指责人 => 对事不对人
尽量疑问,不要太肯定 ,因为可能忽视了上下文
发现问题, 尽量提供建议
不要"高高在上”
不要吝啬于称赞
2.3.2 冲突解决
不要踢皮球, 尽量达成共识
寻求第三人评估
模块owner敲定
组内讨论
2.4 适度的容忍
2.4.1 没必要力求完美
不要让不完美成为拒绝的理由
优先处理更重要的事情
通常来说90分就够用了
比如:
英文语法错误
缺少极端情况下的处理
极致性能的要求