一、Code Review 简述
为保证上线代码质量,经研究决定0412版本起实行Code Review 。具体操作方式为组织 review 会。提出的优化点需立即执行更改,Review会要求给出调整方式方法。同时为了确保项目或迭代版本的时间,请各开发同学提前做好时间规划。此流程为试运行流程,在团队操作过程中 Code Review 方式方法会随之进行优化调整。
这里是来自维基百科的官方解释,CR,全称 Code Review,中文名 - 代码审查,其目的是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术。代码审查常以不同的形式进行,例如结对编程、非正式的过整个代码,或是正式的软件检查。
二、Code Review 目的
- 交叉排查缺陷:通过团队成员相互审核,避免代码层面出现显而易见的问题
- 提高代码质量:通过团队成员相互监督,在完成功能的基础之上不断完善代码结构
- 建立团队意识:代码是团队财产,团队成员在相互督促与改进中共同成长
Code Review 也就是代码评审。代码评审有两种不同的方法,
一种是代码审查(比较正式)。
一种是代码组查(没那么正式)。
之所以需要代码评审,是因为通常自己对自己写的代码都难以发现问题,因此需要以第二双眼睛再次检查代码,帮助我们及时地发现潜在的问题。
1.确保整体代码标准化统一,在一定程度上避免编程引发的基础原则性问题
2.提高研发质量,为提测和交付上线提供双重保障
3.在满足审核标准的情况下,使PHP组整体代码健康状况随着时间的推移而改善
4.审核代码的质量,比如可读性、可维护性、以及基础程序的逻辑和风险点,针对业务逻辑的正常实现不进行深度review
三、Code Review GitLab操作流程
具体操作分支
第一步分支 test 更改为 保护分支,只有指定组长才有权限进行合并操作。具体开发人员只能指定人员发起合并请求。
具体操作通过 Gitlab平台主要Merge-Request机制进行代码审查
1. 开发人员(项目成员)在本地完成一个任务(编码)并通过本地单元测试(自测)。
2. 同步源仓库内容(release_product_hw)至本地仓库「如果跨版了,合并最新代码到本地分支」:可能需要解决冲突,再次进行本地单元测试,通过后push至个 人gitlab远程库。
3. 登录Gitlab,进入目标仓库(Project,后续统称目标仓库),发Merge Request,详见下面截 图:
选择准备申请合并的 Source Branch 与 Target Branch ,如下图:
输入此次合并的关键信息(功能、解决的问题等),指定代码审核人员并提交Merge-Request。提 交后Gitlab将会邮件通知相关的代码审核人员。
4. 相关人员收到邮件通知后登录Gitlab对M-R请求进行处理,也就是进行代码评审。
如果对准备提交的代码不满意,则可以在 有问题的代码行 或 讨论区 给出意见或建议。
如果觉得本次M-R的代码没问题,直接在评论区回复:通过 。并进行合并操作。若没有 Push 权限,则将此M-R请求 assign 给有 Push 权限的人。
5. 在上一步中,如果代码评审未通过,开发人员(M-R申请者)需要根据评审意见进行修改 (当然是得在本地开发环境进行修改测试),即重新根据 步骤1、步骤2 进行操作,最新的提 交信息会实时同步至此前提交的M-R申请单的信息流中。
6. 代码审查流程重新跳回 步骤4 。
7. 具有 Push 权限的开发人员收到M-R请求后,如果讨论区的回复内容为: 通过 ,则执行 Merge 操作。至此整个代码审查流程结束。
M-R申请者也可以提前关闭M-R请求:自己事先发现代码问题等。
如果错误点击了Merge操作,也可以 reopen 。
四、Code Review 时间轴
由 开发人员 根据开发情况,向即将上线版本(由发版人指定)分支发起 Merge Requests 请求,然后通知相关人进行 CodeReview,功能较大更改文件较多的,组织开 Code Review 会议,确定时间、地点、相关人员。
往往发生的实际场景是项目时间比较紧,尽管提出了问题,但是没有细究,CR 直接快速过了。后面果然有问题,花费了两三天去排查解决,整个模块几乎重写。
通常来说:开发 + CR讨论时间 < 不 CR,遇到问题后面修改,设计不合理的维护成本。所以这里的一个点,前期就接受他的存在,并将它计算在排期中。
场景说明:
你在认真的写代码,然后旁边的小菜开心的喊你帮我看个 CR。作为一个认真负责的你,当然是满口答应,准备点进去看看,过了一秒钟打开GIT网页,发现 365 个文件变更,此时求你的心里阴影面积。
这种场景,其实是一次性囤的代码太多导致的,很多同学觉得,我应该把一个功能完整的开发完毕,再提上去,大家才知道我在写什么,但是实际上,coding 是个循序渐进的过程,建议日常开发的同学控制 commit 粒度,尽可能保证每天提交,以及尽可能写好 commit message,还有就是 commit 跟卡片做好关联,也有助于 reviewer 能更好的理解你的意图。
五、PHP 代码规范参考
【知识整理】PHP研发组代码规范要求-CSDN博客
六、常规Review List
1.测试代码类
- podfile、podspec是否修改了本地仓库路径
- 测试类代码,假数据 需要移除
2.版本计划类
- 是否有不在此次版本计划内的逻辑被错误提交
- 是否有偏离本次需求的其他模块代码被误修改
3.数据安全类
- 空安全
- 数组、字典安全
- 线程安全
- 内存安全
4.架构设计类
- 按照组件化标准,基础组件功能不能冗余在业务里;业务不能冗余在基础组件里
- 鲁棒性满足基本要求
- 可抽象的重复基础逻辑
5.审核合规类
- 用户隐私明文传输
- 未加入用户协议隐私协议的隐私功能
- 保证最新版本,低版本兼容性
- 减少使用即将过期,废弃的api
- 敏感词屏蔽
- 其他已知政策风险
6.可读性类
- 不使用特殊字符表情中文
- 建议过长的方法需要拆分
- 复杂难懂方法需要添加注释 (适当注释,言简意赅)
七、业务型必要Review
增加新老版代码偏移量检测
- 通过代码修改对比,发现主Type类型相关逻辑修改,需要研发二次确认。
(例如旧代码type=0跳转详情页,新代码type=1跳转详情页,类似情况需要找研发再次确认逻辑)
- 通过代码修改对比,发现城市ID逻辑有大量修改,需要研发二次确认。