code review的一些小技巧

最新推荐文章于 2025-11-05 23:06:37 发布

原创最新推荐文章于 2025-11-05 23:06:37 发布 · 1.5k 阅读

11 ·

CC 4.0 BY-SA版权

文章标签：

#代码复审

部署运行你感兴趣的模型镜像

概述

Code Review在整个研发流程的重要性相信每位研发同学应该都比较了解，这里就不再多强调。总结起来CR有以下几大收益：

增加代码规范的主动性（心里暗示别人会Review，所以会注意规范）
代码复查、提升质量
代码分享、知识共享
新人培养、言传身教

但是如果Code Review的姿势做的不正确，同样也会有负面影响：

流于形式，导致没有发挥CR应有的作用
没有统一的指引，导致CR的质量不高
CR评论意见不恰当，被Review的作者不乐意接受Review

目标

提高代码质量，降低线上故障率
提高研发效率，降低研发成本
知识共享，文化培养

名词解释

名词	解释
CR	code review
CL	changelist，指本次改动
reviewer	review CL的人
author	提交changelist的人
LGTM	Looks good to me简称，一般表示代码符合团队标准，CL获得通过，允许被合并
nit	nitpick简称，表示鸡蛋里挑骨头的意思

软件分支模式

业界常用的软件分支模式有多种，但本质上可以分为两类：

主干开发模式（Trunk Based Development）
特性分支开发模式（Feature Branch Development）

分支模式	定义	优点	缺点	适用场景	使用公司用例
特性分支开发模式	特性分支模式是指为一个或多个特定需求/缺陷/任务创建代码分支，在其上完成相应的开发后，把它合并到主干/集成分支的开发模式	特性开发周期宽松分支测试的时间宽松	分支管理复杂合并冲突多迭代速度慢需要多测试环境	对版本迭代速度要求不高测试自动化程度低，或者主要人工测试	VIPVivomidea
Trunk Based Development	是指开发人与那直接向主干提交/推送代码，待到达发布条件时从主干拉出发布分支，用于发布，若发现缺陷，也在主干分支上修复，并根据需要cherry-pick到对应版本的发布分支	分支模型简单高效，开发人员容易掌握避免分支合并、冲突解决的困扰随时拥有可发布的版本有利于持续集成和持续交付	依赖的基础架构要求高/工具体系完善自动化测试要求高最好有代码评审最好有特性开关	对迭代速度要求高，希望需求快速交付基础架构强，持续集成工具高效团队成员习惯TDD，代码自动化测试覆盖率高	googletencent

结合media研发流程现状，当前阶段优先采用特性分支开发模式，下面CR的流程业主要基于特性分支模式讲解。

CR流程

单个feature多人开发分支

适合完成一个较大的功能需求，多人协作场景，团队内互相CR。

单个feature单人开发

单人开发适合小bugfix，配置修改，一般代码行数不超过100行，在测试环境验证通过后再进行CR合并到主干分支，分支管理细节参考《git分支规范》。

常见的几种CR方式比较

方式	优点	缺点
个人或特性分支合并集成分支时CR	单词CR的代码数量少，单次CR的时间较少，CR速度快有效锁定变更内容，效果更有保障CR质量可量化	若CR保持较高频率需耗费较多的人力资源
线下会议集中CR	耗费资源较少，不需要特别工具支持更多的人同时参与CR，增加了找出更多问题可能性	单次CR的代码量较大，效果难以保障CR过程中速度较慢，单次CR可能需要半天将reviewer同时集中较难，多人容易在时间上冲突CR的时机一般较晚，频率较低，CR找出来的问题通常不能得到的充分的时间进行修改

CR应该做什么？

CR指引

参考：https://google.github.io/eng-practices/review/reviewer/looking-for.html

各层面主要关注点：

在这里插入图片描述

CR过程

在这里插入图片描述

参考：https://google.github.io/eng-practices/review/reviewer/navigate.html

工具

gitlab merge request, pull request
gerrit
CodeSource

CR common check list

规范：
- 规范是否被遵守？
实现：
- 功能是否正确完成?是否正确达到相应的目的？
- 是否有夹带功能或者缺失功能点？
- 是否增加了新的依赖，依赖是否必要？
- 是否依赖了新的第三方库，第三方库是否有经过内部讨论是否合适？
- 代码分层是否合理？
- 是否在重复发明轮子？
复杂性
- 类、函数（方法）设计是否过于复杂？过于复杂是指读者不能快读理解代码意图
- 是否存在过度设计? 过度设计是指暂时不需要的过于通用性（generic）设计，以及添加了目前不需要的方法
- 是否存在过早性能优化（performance hacker）？
测试：
- 代码有测试用例吗？
- 测试用例是否覆盖CL核心变更代码？
- 测试代码是否大量重复？是否需要抽象？
- 边界值和异常输入是否覆盖？
异常：
- 是否有必要抛出异常？
- 是否漏掉了一些异常的处理？
- 异常梳理是否进行了分类?
- 是否吞并了异常？
- 错误码是否符合规范？
- 异常类型是否正确？
- 异常是否一定会阻塞主流程（fail-fast）？可否降级或者fail-back？
日志：
- 日志是否清晰？
- 日志级别是否正确？
- 是否丢失上下文？
- 日志是否多余？
- 日志打印信息是否考虑了国际化和编码问题？
配置：
- 配置是否遵守规范？
- 配置是否有默认值？
- 敏感配置是否与代码分离？
注释：
- 代码注释应该描述为什么会出现这段代码，而非这段代码做了什么，代码本身应该简单（self-explaination）,文件、类、函数注释应该描述目的、如何使用，以及行为
- 是否存在多余注释（是否必要）？
- 注释是否过期？
- 复杂算法是否有合理解释？
- 是否存在TODO/FIXME 或者其他？
文档：
- 如果CL更改了构建、测试、交互、发布的时候，请检查是否也同时更新相关文档，包括README，generated doc；
- 如果CL 删除或弃用了一些代码，请考虑是否应该删除对应文档
性能：
- 内存是否可优化？是否存在未正确终止的循环？
- 是否存在获取数据范围过大（比如查询为未使用分页）？
- 是否存在并发安全风险，临界区是否被保护？
- 锁资源是否被正确释放？
- 锁粒度是否可以优化？
- 锁时长是否可缩短？
- 是否存在deadlock？
- 加锁顺序是否正确？
安全：
- 是否需要进行身份验证？
- 敏感信息是否直接hard code代码中？
- 日志中是否输出了敏感信息？
- 加密算法是否有被正确使用？
- 外部input是否被checked？返回值是否被checked？
- 是否存在字段长度溢出？

参考：midea代码规范（java/mysql/api）

CR规范/原则/注意事项

【必须】所有的需要发布生产的代码提交都要得到CR，代码经过CR后才能merge到核心分支（特性分支或者master分支）

【必须】MR的description必须清晰，让reviewer更快get到作者意图

【必须】CR通过之后，通过MR采用squash的方式Merge到核心分支

【必须】CR基于MR的基础进行review，因为普通的commits比较零散，较难做CR

【必须】功能代码和UT一起作为一个MR，进行CR，对于UT 和功能代码同等对待

【必须】避免一次提交多个功能点，多个功能点会导致reviewer关注点不明确

【必须】CR的reviewer不能指定为自己，reviewer的选择上应该是能够给你快速CR的peer

【必须】对代码不对人，针对代码本身做CR。reviewer在委婉拒绝该CL的同时也要提供替代方案，而且仍然保持礼貌，我们希望即使不同意也会相互保持尊重

【必须】代码风格选择取决软件设计标准原则，若不是在midea代码规范中的一些点（code style）被视为个人偏好，若reviewer不能证明比作者的方法更好，请尊重作者的个人偏好

【建议】做CR时，多用疑问句，少用肯定句

【建议】对于写的好的代码，也可以给与称赞和表扬

【建议】CR优先指定熟悉提交代码相关功能的其他开发人员

【建议】一般建议小步快跑的方式，每个CR的变更代码行数一般为200行以内俱佳，尽量不要超过500，否则会降低CR的效果

【建议】开发人员应该每天提交一次CL（如有代码交付物）， CR应保持尽可能快地完成，最迟不超过一个工作日

【建议】没有“完美”的代码，只有持续改进

【建议】reviewer想表达一些更好的东西，但不是很重要，可以在comment前标注“nit：”，表明作者不必在本次CL修改它

【建议】在某些情况下（参考：https://google.github.io/eng-practices/review/emergencies.html#what），reviewer可以给与LGTM甚至approval，即便CL还有一些comment未处理

【建议】在CR冲突时，首先是尽量通过本指引来达成共识，当很难达成共识时，应该面对面会议解决冲突，而不仅仅是通过在CL中添加注释来解决

CR技能提升

为提升全员的CR技能，提高代码review质量，不定期的安排一些代码CR的分享会

**CR周报和CR月报：**定期发送CR周报和月报，主要内容为各组，各人员的CR情况汇总。
**看板反馈CR质量：**看板直接显示CR的覆盖率和comment数据指标，同时手工不定期的抽查CR的评审记录和质量。一方面审阅reviewer的CR质量；另一方面也对author的常见问题汇总分类
**CR最佳奖：**定期评审出CR质量最佳的前几名，并给与相应奖励。
CR实例分享会： 对于一些优秀和典型的CR，会举办线上线下的分享会，通过实例分享一些做的优秀的CR，安排如下

CR分享会
参会人	reviewer，reviewee、开发人员
分享主题	实例CR
分享实例	代码适中的（200_{400行）的CR案例，大约能在30}45min内分享完毕
分享过程	由reviewer对代码进行讲解，同时对每一个CR的comment进行讲解和点评，同时指出开发过程中注意的问题
分享总结	reviewee总结评审要点，记录到wiki中，以方便后续知识的共享