跳到主要内容

代码审查速查表

快速参考代码审查的关键要点,帮助审查者高效、系统地进行代码审查。

审查前准备

作为代码作者

  • 代码能够编译/运行通过
  • 所有测试通过
  • 静态检查无错误
  • 编写清晰的 PR 描述
  • 自测边界情况

作为审查者

  • 理解变更背景和目的
  • 阅读相关需求文档
  • 准备审查清单
  • 安排充足的时间

快速审查清单

功能性

检查项优先级说明
功能实现正确🔴 高代码实现了预期的功能
边界条件处理🔴 高空值、越界、异常输入等
错误处理完善🔴 高异常捕获、错误信息、资源释放
并发安全🟡 中多线程/并发场景处理

代码质量

检查项优先级说明
命名清晰🔴 高变量、函数、类名表达意图
函数长度适中🟡 中建议不超过 50 行
避免深层嵌套🟡 中建议不超过 3 层
单一职责🔴 高函数/类职责单一
消除重复🟡 中DRY 原则

设计

检查项优先级说明
架构合规🔴 高符合项目架构规范
依赖合理🟡 中模块间依赖关系
接口设计🟡 中开闭原则、接口隔离
扩展性🟢 低易于扩展和维护

安全性

检查项优先级说明
SQL 注入防护🔴 高使用参数化查询
XSS 防护🔴 高输出编码、CSP
输入验证🔴 高所有外部输入验证
敏感信息保护🔴 高密码、密钥等
权限控制🔴 高正确的认证授权

性能

检查项优先级说明
算法复杂度🟡 中时间/空间复杂度合理
数据库优化🟡 中索引、查询优化
资源释放🔴 高连接、句柄正确关闭
缓存使用🟢 低合理使用缓存

测试

检查项优先级说明
测试覆盖🔴 高核心逻辑有测试
边界测试🔴 高边界条件有测试
错误测试🟡 中错误场景有测试
测试质量🟡 中测试独立、可维护

意见分级标记

标记含义处理方式
🔴 阻塞必须修复修复后才能合并
💡 建议建议采纳讨论后决定是否修复
📝 可选仅供参考可选修复
❓ 疑问需要澄清需要开发者解释
👍 好评好的实现给予肯定

常见代码问题速查

命名问题

// ❌ 不好的命名
int d; // elapsed time in days
int elapsedTime;

// ✅ 好的命名
int elapsedTimeInDays;
int daysSinceCreation;

函数过长

// ❌ 函数过长(超过 50 行)
public void processOrder(Order order) {
// 100+ 行代码...
}

// ✅ 拆分为小函数
public void processOrder(Order order) {
validateOrder(order);
calculateTotal(order);
saveOrder(order);
sendNotification(order);
}

深层嵌套

// ❌ 深层嵌套
if (user != null) {
if (user.isActive()) {
if (user.hasPermission()) {
// do something
}
}
}

// ✅ 提前返回
if (user == null) return;
if (!user.isActive()) return;
if (!user.hasPermission()) return;
// do something

魔法数字

// ❌ 魔法数字
if (status == 3) {
// do something
}

// ✅ 命名常量
private static final int STATUS_COMPLETED = 3;

if (status == STATUS_COMPLETED) {
// do something
}

异常处理

// ❌ 忽略异常
try {
process();
} catch (Exception e) {
// ignore
}

// ✅ 正确处理
try {
process();
} catch (IOException e) {
logger.error("Process failed", e);
throw new BusinessException("处理失败", e);
}

SQL 注入

// ❌ 不安全的 SQL
String query = "SELECT * FROM users WHERE id = " + userId;
stmt.executeQuery(query);

// ✅ 参数化查询
PreparedStatement ps = conn.prepareStatement(
"SELECT * FROM users WHERE id = ?"
);
ps.setInt(1, userId);

语言特定速查

Java

检查项说明
Optional处理可能为空的值
Stream API集合操作
try-with-resources资源管理
泛型类型安全
不可变集合List.of(), Set.of()

Python

检查项说明
类型注解def func(x: int) -> str
列表推导式[x for x in items if x > 0]
with 语句资源管理
生成器yield 处理大数据
f-string字符串格式化

JavaScript/TypeScript

检查项说明
async/await异步处理
解构赋值const { a, b } = obj
模板字符串`Hello ${name}`
可选链obj?.property
空值合并value ?? default

Go

检查项说明
错误处理if err != nil
Context取消和超时
defer资源释放
接口隐式实现
Goroutine并发安全

审查意见模板

阻塞性问题

🔴 阻塞:[问题描述]

当前代码:

[代码片段]


建议修改为:

[修复后的代码]


原因:[解释为什么这是问题]

建议性问题

💡 建议:[建议描述]

可以考虑:

[改进代码]


好处:[解释改进的好处]

可选性问题

📝 可选:[建议描述]

仅供参考,不强制修改。

问题澄清

❓ 疑问:[问题]

能否解释一下 [具体部分] 的设计考虑?

好评

👍 这个实现很好!

特别是 [具体部分],[原因]。

审查时间参考

PR 大小建议审查时间审查速度
< 100 行15-30 分钟200 行/小时
100-300 行30-60 分钟150 行/小时
300-500 行60-90 分钟100 行/小时
> 500 行建议拆分< 100 行/小时

响应时间规范

场景响应时间
收到审查请求24 小时内
紧急修复2 小时内
审查意见回复24 小时内
阻塞性问题修复优先处理

审查文化检查

  • 对事不对人,评论针对代码
  • 提供具体、可操作的建议
  • 解释问题原因和改进好处
  • 认可好的代码和设计
  • 保持开放心态,接受不同意见
  • 及时响应,避免阻塞
  • 区分问题优先级
  • 记录决策和理由

AI 辅助审查要点

AI 审查工具对比

工具平台支持特点适用场景
GitHub CopilotGitHub深度集成、一键应用建议GitHub 用户首选
CodeRabbitGitHub/GitLab/Bitbucket多平台、深入分析跨平台团队
Cursor BugBotCursor IDE实时检测、IDE 集成Cursor 用户

VS Code 审查模式

模式功能配额消耗自定义指令
审查选区审查选中代码片段不消耗不支持
审查变更审查所有未提交变更消耗 1 个高级请求支持

AI 审查配置要点

自定义指令位置

  • 仓库级别:.github/copilot-instructions.md
  • 路径特定:.github/instructions/**/NAME.instructions.md

配置示例

# .github/copilot-instructions.md

## 审查重点
- 用中文回复
- 检查安全漏洞(SQL 注入、XSS)
- 关注性能问题(N+1 查询、循环中数据库操作)

## 输出格式
- 🔴 Critical: 必须修复
- 🟡 Warning: 建议修复
- 🟢 Info: 可选建议

## 已有工具覆盖(可忽略)
- 代码格式(Prettier)
- 代码风格(ESLint)

编写有效指令的要点

要点说明
保持简短单个文件不超过 1000 行
结构清晰使用标题、项目符号、祈使句
提供示例包含正确和错误模式的代码片段
区分级别仓库级用于通用规则,路径特定用于语言/框架规则

AI 审查最佳实践

实践说明
设置置信度阈值只显示高置信度问题,减少噪音
告知 CI 覆盖告诉 AI 哪些问题已由 CI 检查
收集反馈对 AI 评论进行点赞/点踩
人机协作AI 先审 → 开发者响应 → 人工终审

AI 审查局限性

AI 擅长

  • 代码格式和风格检查
  • 常见 bug 模式检测
  • 安全漏洞扫描
  • 性能问题建议

AI 不擅长

  • 业务逻辑正确性验证
  • 架构设计决策
  • 用户体验评估
  • 复杂的系统交互

主要风险

  • 遗漏问题:大型或复杂变更可能遗漏问题
  • 假阳性:可能报告不存在的问题(幻觉)
  • 代码建议风险:提供的代码可能不准确或不安全
  • 潜在偏见:训练数据可能包含偏见

关键提醒:AI 审查是补充而非替代人工审查,始终验证 AI 的反馈!

审查决策流程

是否批准的判断流程

收到 PR


┌─────────────────────────────┐
│ 是否存在安全问题? │
│ (SQL注入、XSS、敏感信息泄露) │
└──────────┬──────────────────┘

┌─────┴─────┐
│ 是 │ 否
▼ ▼
阻止合并 ┌─────────────────────────────┐
│ 核心功能是否正确实现? │
│ (业务逻辑、边界条件) │
└──────────┬──────────────────┘

┌─────┴─────┐
│ 否 │ 是
▼ ▼
阻止合并 ┌─────────────────────────────┐
│ 是否有测试覆盖? │
│ (核心逻辑需要测试) │
└──────────┬──────────────────┘

┌─────┴─────┐
│ 否 │ 是
▼ ▼
要求补充测试 ┌─────────────────────────┐
│ 代码健康状况是否改善? │
│ (可读性、可维护性提升) │
└──────────┬──────────────┘

┌─────┴─────┐
│ 否 │ 是
▼ ▼
讨论改进 ✅ 批准合并
后再决定 (可留建议性评论)

批准原则(来自 Google 工程实践)

根据 Google 的标准:只要代码确实改善了系统的整体健康状况,就应该批准,即使它不完美。

可以批准不应该批准
代码比之前更好代码健康状况下降
建议性问题已记录存在未解决的阻塞性问题
测试覆盖充分关键测试未通过
符合团队规范存在已知安全漏洞

常见问题诊断表

问题:PR 长时间无人审查

可能原因解决方案
没有明确指定审查者使用 CODEOWNERS 自动分配
审查者太忙分散审查负载,多人参与
PR 过大拆分为多个小 PR
领域不熟悉指定领域专家 + 其他审查者

问题:审查意见引起争论

可能原因解决方案
沟通方式不当对事不对人,提供理由
标准不明确建立团队审查规范文档
设计偏好差异基于原则讨论,必要时升级
信息不对称面对面会议,记录讨论结果

问题:审查流于形式

可能原因解决方案
审查者疲劳轮换审查者,控制审查量
时间压力强调审查的价值和重要性
能力不足培训,使用检查清单
缺乏反馈分享高质量审查案例

问题:频繁返工

可能原因解决方案
提交前自检不足强化自检清单
PR 描述不清晰使用 PR 模板
需求理解偏差提前沟通,明确需求
审查标准不一团队统一审查标准

审查优先级速查

必须阻止合并

  • 🔴 安全漏洞(SQL 注入、XSS、CSRF)
  • 🔴 功能缺陷(逻辑错误、数据处理错误)
  • 🔴 数据丢失风险
  • 🔴 性能严重问题(内存泄漏、死锁)
  • 🔴 破坏向后兼容性

建议修复后合并

  • 🟡 缺少测试覆盖
  • 🟡 代码可读性问题
  • 🟡 潜在的性能优化
  • 🟡 重复代码
  • 🟡 异常处理不完善

可以留待后续

  • 🟢 代码风格微调
  • 🟢 非关键的命名改进
  • 🟢 文档更新(非紧急)
  • 🟢 技术债务清理

不同场景的审查重点

新功能开发

重点关注检查要点
功能正确性是否满足需求?边界条件?
测试覆盖是否有单元测试?集成测试?
文档更新README、API 文档是否更新?
向后兼容是否影响现有功能?

Bug 修复

重点关注检查要点
根因修复是否解决根本原因?
回归测试是否添加测试防止复发?
影响范围是否影响其他功能?
日志记录是否便于问题追踪?

重构

重点关注检查要点
行为保持功能是否保持不变?
测试通过所有测试是否通过?
增量变更是否拆分为小步骤?
清理彻底是否删除了废弃代码?

紧急修复

重点关注检查要点
快速有效是否解决问题?
最小变更变更范围是否最小?
后续处理是否有后续完善计划?
通知相关相关方是否知情?

快速自检清单

提交 PR 前(作为作者)

□ 代码能够编译/运行
□ 测试通过
□ 静态检查无错误
□ PR 描述清晰
□ 变更范围适中(< 400 行)
□ 自测边界情况
□ 无敏感信息泄露
□ 相关文档已更新

审查 PR 时(作为审查者)

□ 理解变更目的
□ 检查功能正确性
□ 检查安全风险
□ 检查性能影响
□ 检查测试覆盖
□ 检查代码可读性
□ 提供具体可行的建议
□ 区分问题优先级

提示:将此速查表保存为书签或打印出来,在代码审查时快速参考。