从混乱到秩序:Code-Review代码审查助力代码质量飞跃。Code-Review,顾名思义,就是对代码进行审查的过程。它不仅仅是简单地检查代码是否存在语法错误或逻辑问题,更是一种团队协作和知识共享的方式。通过Code-Review,团队成员可以相互学习、相互借鉴,共同提升代码质量。
🧑 作者简介:现任阿里巴巴嵌入式技术专家,15年工作经验,深耕嵌入式+人工智能领域,精通嵌入式领域开发、技术管理、简历招聘面试。CSDN优质创作者,提供产品测评、学习辅导、简历面试辅导、毕设辅导、项目开发、C/C++/Java/Python/Linux/AI等方面的服务,如有需要请站内私信或者联系任意文章底部的的VX名片(ID:gylzbk)
💬 博主粉丝群介绍:① 群内高中生、本科生、研究生、博士生遍布,可互相学习,交流困惑。② 热榜top10的常客也在群里,也有数不清的万粉大佬,可以交流写作技巧,上榜经验,涨粉秘籍。③ 群内也有职场精英,大厂大佬,可交流技术、面试、找工作的经验。④ 进群免费赠送写作秘籍一份,助你由写作小白晋升为创作大佬。⑤ 进群赠送CSDN评论防封脚本,送真活跃粉丝,助你提升文章热度。有兴趣的加文末联系方式,备注自己的CSDN昵称,拉你进群,互相学习共同进步。
🗄️ 专栏介绍:本文归属于专栏《嵌入式开发流程》,专注嵌入式硬件产品开发中的流程性知识,持续更新中,欢迎大家免费订阅关注。
从混乱到秩序:Code-Review代码审查助力代码质量飞跃
- 1. 概述
- 2. Code-Review审查项
- 2.1 编码格式
- 2.2 commit message检查
- 2.3 patch原子性检查
- 2.4 新增函数是否有函数头
- 2.5 新增文件是否有文件头
- 2.6 新增头文件和源文件匹配
- 2.7 重名宏重复定义
- 2.8 看到if就去找else
- 2.9 看到malloc就去找free
- 2.10 看到while/for就去找边界条件
- 2.11 看到数组去找边界
- 2.12 看见内存拷贝关注长度
- 2.13 看到锁就去找解锁
- 2.14 看见return就去前面找资源
- 2.15 变量初始值
- 2.16 栈内内存分配控制(一般用于RTOS系统)
- 2.17 函数体规模不宜太大
- 2.18 圈复杂度不能太高
- 2.19 业务逻辑疑问和模块owner确认
- 2.20 针对C++特性
- 2.21 SOLID原则遵循
- 2.22 想想哪里还能再优化
- 3. 获取checklist
- 4. 总结
1. 概述
对于一个嵌入式项目来说,代码质量往往决定了项目的成败。然而,随着项目规模的扩大和团队成员的增多,代码质量的问题也将日益凸显。混乱的代码结构、难以理解的逻辑、潜在的安全隐患……这些问题都可能成为项目推进和质量保障的绊脚石。而Code-Review,作为一种重要的代码质量控制手段,正逐渐被越来越多团队所重视。
Code-Review,顾名思义,就是对代码进行审查的过程。它不仅仅是简单地检查代码是否存在语法错误或逻辑问题,更是一种团队协作和知识共享的方式。通过Code-Review,团队成员可以相互学习、相互借鉴,共同提升代码质量。
2. Code-Review审查项
那么,Code-Review究竟如何助力代码质量飞跃呢?接下来,我将基于我十几年工作生涯中的Code-Review经验,为大家一一揭秘,希望能帮助大家在以后的代码开发过程中,逐步提升代码质量,促进项目顺利上线。
2.1 编码格式
审查方法
首先,公司内部所有开发人员协商确定好一份大家都认可的编码风格,形成一份开发人员的《代码开发规范SOP》。内容可以包含编码风格及其一些其它共识性的规则,比如:
- if-else即使只有一行代码,也必须加大括号
- 代码缩进使用4个空格而不是Tab
- 所有变量定义时必须进行初始化
- 废弃代码建议直接删除而不是注释掉
- …
- …
- …
处理方法
不符合SOP的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.2 commit message检查
审查方法
代码审查者,要能真实轻松看明白本次commit的代码是在做什么:
- “fix bug/error on helloword”这类无价值的通用性描述语句禁止合入,要求必须准确描述本次commit的意图。
- message格式不合规的禁止合入,比如message过长要求提交人将其拆分为message title和message content,两者中间用空行间隔开。详细冗长的描述可以放在message content中。
- message和patch内容有出入的禁止合入;
- message缺少必要验证结果呈现的禁止合入;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.3 patch原子性检查
审查方法
1次提交是否只做了一件事。
处理方法
1次提交做了多件事的,要求拆开后分多次提交。
2.4 新增函数是否有函数头
审查方法
- 本模块对外接口必须要函数头,
static
和inline
的根据实际需要; - 函数头格式符合公司和团队要求,参考:
/**
* @brief The handle any demux operation that could not be expressed by other functions.
* @param[in] this The reflexive to the vma_demux_plugin itself.
* @param[in] private The points to plug-in private data.
* @param[in] cmd The command word to specific control operation.
* @param[inout] arg The argument related to cmd.
* @return CONTROL_TRUE, CONTROL_SUCCESS, CONTROL_FAILURE, CONTROL_FALSE, CONTROL_UNKNOWNCMD.
*/
- 更新参数列表、函数名、返回值、核心逻辑时对应更新函数头描述;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.5 新增文件是否有文件头
审查方法
新增源文件必须写文件头,格式符合公司和团队要求,参考:
/**
* Copyright (C) 2020 替换为公司名, All rights reserved.
*
* @file 替换为文件名
* @brief 替换文文件的功能描述
* @author 替换为文件创建作者的名字或者邮箱
* @date 替换为文件创建日期
*/
注意:
- Copyright的年份要写成当年;
- 头文件必须做重复包含防御;
- 重构时根据需要更新文件头;
- 文件权限合理;
示例如下:
/**
* Copyright (C) 2020 Alibaba.inc, All rights reserved.
*
* @file main.h
* @brief The main head file for nshell.
* @author this_is_a_demo@alibaba-inc.com
* @date 2020/11/27
*/
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.6 新增头文件和源文件匹配
审查方法
- 原则上功能模块每个cpp/c文件要有一个对应的头文件,特殊情况可让步;
- 头文件不能循环include;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.7 重名宏重复定义
审查方法
C文件的重名宏定义需要先undef;用于避免编译warning。
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.8 看到if就去找else
审查方法
- 条件路径封闭检查,是否存在不在条件全路径内的场景;
- 条件分支太多建议改成
switch-case
语句,或者其他更清晰的实现;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.9 看到malloc就去找free
审查方法
- 是否没有free;
- 是否存在重复free路径;
- free后指针是否置NULL;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.10 看到while/for就去找边界条件
审查方法
循环退出的边界处理是否妥当,存不存在死循环等异常路径;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.11 看到数组去找边界
审查方法
- 数组起始边界一般都能注意到,结束边界容易疏忽;
- 数组名退化为指针时注意用法,数组名不能做左值;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.12 看见内存拷贝关注长度
审查方法
- 源和目的地址范围匹配;
- 字符串处理函数使用带
n
版本,如strncpy
; - 环形拷贝注意position,避免回卷覆盖引发错误;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.13 看到锁就去找解锁
审查方法
- 锁上下文是否太大,只针对必要的上下文上锁;
- 找找解锁,避免死锁;
- 思考当前上下文中上锁是否有其他业务风险;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.14 看见return就去前面找资源
审查方法
返回前关注资源回收处理,如解锁、json对象释放等;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.15 变量初始值
审查方法
所有变量定义必须赋初值,避免可能出现的未初始化访问带来异常结果。
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.16 栈内内存分配控制(一般用于RTOS系统)
审查方法
- 不能在栈内开辟大空间,防止爆栈;
- 超过1K的buffer不要放在函数栈内,比如
int arr[2048];
;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.17 函数体规模不宜太大
审查方法
- 函数体非空非注释的有效行数不超过100行;(google给的是40,我觉得小了点)
- 重复语句太多看看能不能去重;
处理方法
超20%以内可以斟酌,20%以上必须解决才能合入;
2.18 圈复杂度不能太高
审查方法
- 函数圈复杂度不超过10;
- 工具扫描最准,简单函数体粗略计算下就可以;
处理方法
拆解或者重写降低圈复杂度后重新提交审查
2.19 业务逻辑疑问和模块owner确认
审查方法
- 方案实现细节是否可以改进;
- 资源使用上能否调整;
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.20 针对C++特性
审查方法
- 内部通用的namespace建议封装成宏;
- using namespace不能放在头文件中,要放在cpp源文
#include
下方; - 其他
处理方法
不符合的,在Code-Review平台评论审查意见,提交人跟踪意见,直到问题解决后,才能允许merge进代码仓库。
2.21 SOLID原则遵循
审查方法
- 上层实现不依赖底层模块,应该依赖于抽象,尽量反转依赖;
- 对扩展开放,对修改封闭,涉及到接口变更必须对齐用法现状;
- 主要在架构设计审评时,一般代码review阶段较少涉及;
处理方法
2.22 想想哪里还能再优化
审查方法
最后六一个开放性审查项,没有具体要求,只是希望审查者在审查代码时深入思考一个问题:“哪里还可以更优化”。
处理方法
自行思考
3. 获取checklist
我已经将上述的审查项整理成excel表格,便于Code-Review过程中逐项检查,快速且高质量完成代码的审查检阅工作。
如有需要,关注我并私信"checklist",我会发送给你excel表格。
4. 总结
要想充分发挥Code-Review的作用,我们还需要注意一些细节。首先,要保持开放和包容的心态。在审查他人代码时,我们要尊重他人的劳动成果,避免过于苛刻或挑剔。同时,我们也要勇于接受他人的意见和建议,不断完善自己的代码。其次,要注重沟通和协作。在Code-Review过程中,我们要积极与团队成员进行沟通和交流,共同讨论和解决问题。通过团队协作,我们可以更好地发现问题、解决问题,提升代码质量。
总之,Code-Review是一种非常有效的代码质量控制手段。通过它,我们可以发现潜在问题、统一编码规范和风格、提升团队成员的技能水平,从而实现从混乱到秩序的蜕变,让代码质量得到飞跃式的提升。因此,我们应该重视Code-Review在软件开发过程中的作用,并积极推广和应用它。