跳到主要内容

审查流程与实践

代码审查的流程决定了审查的效率和效果。一个设计良好的流程能够在保证质量的同时，避免成为开发瓶颈。本章将详细介绍如何建立高效的代码审查流程。

准备阶段：提交前的自检​

在提交代码审查之前，开发者应该进行充分的自我检查。这不仅能减少审查者的工作量，也能培养开发者对自己代码负责的态度。

代码自检清单​

功能验证

确保代码实现了预期的功能，并且通过了所有相关测试：

# 运行单元测试
npm test

# 运行集成测试
npm run test:integration

# 检查测试覆盖率
npm run test:coverage

静态检查

在提交前运行静态分析工具，提前发现并修复常见问题：

# JavaScript/TypeScript 项目
eslint src/
prettier --check src/

# Java 项目
./mvnw checkstyle:check

# Python 项目
flake8 src/
black --check src/

代码审查自测

开发者应该像审查者一样审视自己的代码：

  • 这段代码是否易于理解？
  • 命名是否清晰表达了意图？
  • 是否有明显的逻辑错误？
  • 边界条件是否处理得当？
  • 错误处理是否完善？

编写清晰的提交信息​

好的提交信息能够帮助审查者快速理解变更的上下文。

提交信息结构：

<type>(<scope>): <subject>

<body>

<footer>

示例：

feat(auth): 添加用户登录功能

实现基于 JWT 的用户认证系统：
- 添加登录/登出 API 端点
- 实现密码加密存储（bcrypt）
- 添加 token 刷新机制
- 集成 Redis 存储会话信息

相关需求：PROJ-123

提交信息规范：

类型说明示例
feat新功能feat(api): 添加用户注册接口
fix修复 bugfix(auth): 修复 token 过期不提示问题
docs文档更新docs(readme): 更新部署说明
style代码格式style: 统一缩进格式
refactor重构refactor(service): 提取公共逻辑
test测试相关test(auth): 添加登录单元测试
chore构建/工具chore: 升级依赖版本

控制变更范围​

小步提交原则

每个审查单元应该足够小，便于审查者理解。建议：

  • 单次变更不超过 400 行代码
  • 一个 PR 只解决一个问题
  • 大型重构分阶段提交

不好的实践：

一个 PR 同时包含：

  • 新功能开发
  • 代码重构
  • 依赖升级
  • 配置文件修改

好的实践：

将上述内容拆分为 4 个独立的 PR，每个都有明确的目标和范围。

提交审查：创建有效的 Pull Request​

PR 描述模板​

一个清晰的 PR 描述能够帮助审查者快速理解变更内容。

推荐模板：

## 变更概述
简要描述本次变更的目的和内容。

## 变更类型
- [ ] 新功能
- [ ] Bug 修复
- [ ] 性能优化
- [ ] 代码重构
- [ ] 文档更新

## 主要变更
1. 变更点 1 的详细说明
2. 变更点 2 的详细说明
3. ...

## 测试情况
- [ ] 单元测试已更新并通过
- [ ] 集成测试已更新并通过
- [ ] 手动测试已完成

## 审查要点
请重点关注以下方面：
1. 某个设计决策是否合理
2. 某个边界条件的处理
3. ...

## 相关链接
- 需求文档：[链接]
- 设计文档：[链接]
- 相关 PR：[链接]

选择合适的审查者​

审查者分配原则：

  • 领域专家：涉及特定模块时，分配给熟悉该模块的开发者
  • 知识传播：让不熟悉该模块的开发者参与审查，促进知识共享
  • 负载均衡：避免总是分配给同几个人，分散审查负担
  • 最少审查人数：关键代码至少需要 2 人审查，一般代码 1 人即可

示例：

审查者分配：
- @alice - 支付模块负责人（必需）
- @bob - 后端开发（建议）
- @charlie - 了解订单系统（可选）

执行审查：系统性检查​

审查顺序​

第一步：理解上下文

在查看代码之前，先阅读 PR 描述和相关文档，了解：

  • 为什么要做这个变更？
  • 变更的范围是什么？
  • 预期的行为是什么？

第二步：整体浏览

快速浏览整个变更，了解：

  • 修改了哪些文件？
  • 变更的规模如何？
  • 是否有明显的架构变化？

第三步：详细审查

逐行审查代码，重点关注：

  • 逻辑正确性
  • 代码质量
  • 设计合理性
  • 安全性
  • 性能影响

审查速度控制​

研究表明，审查速度会显著影响发现问题的数量：

审查速度问题发现率建议
< 100 行/小时高适合关键代码
100-300 行/小时中等一般代码的标准速度
> 300 行/小时低仅适合简单变更

建议：

  • 关键代码（支付、安全相关）：< 100 行/小时
  • 一般业务代码：100-200 行/小时
  • 简单变更（配置修改、文档更新）：可适当加快

使用检查清单​

系统性的检查清单能够确保不遗漏重要检查点。

通用检查清单：

## 功能正确性
- [ ] 代码实现了预期的功能
- [ ] 边界条件处理得当
- [ ] 错误处理完善
- [ ] 输入验证充分

## 代码质量
- [ ] 命名清晰准确
- [ ] 函数职责单一
- [ ] 代码易于理解
- [ ] 注释必要且准确

## 设计合理性
- [ ] 符合项目架构规范
- [ ] 依赖关系合理
- [ ] 遵循设计原则

## 安全性
- [ ] 无 SQL 注入风险
- [ ] 无 XSS 风险
- [ ] 敏感信息处理得当
- [ ] 权限控制正确

## 性能
- [ ] 无明显的性能问题
- [ ] 资源使用合理
- [ ] 算法复杂度合适

## 测试
- [ ] 测试覆盖充分
- [ ] 测试用例合理
- [ ] 边界情况有测试

提出审查意见​

意见分级​

阻塞性问题（Blocking）

必须修复才能合并的问题：

  • 功能缺陷
  • 安全漏洞
  • 性能严重问题
  • 破坏向后兼容性

标记方式：

🔴 阻塞：这里存在 SQL 注入漏洞，必须使用参数化查询。

建议性问题（Suggestion）

建议采纳但不是必须的问题：

  • 代码优化建议
  • 设计改进建议
  • 更好的实现方式

标记方式：

💡 建议：这里的循环可以替换为 Stream API，代码会更简洁。

// 当前实现
List<String> result = new ArrayList<>();
for (User user : users) {
    if (user.isActive()) {
        result.add(user.getName());
    }
}

// 建议改为
List<String> result = users.stream()
    .filter(User::isActive)
    .map(User::getName)
    .collect(Collectors.toList());

可选性问题（Nitpick）

仅供参考的小问题：

  • 代码风格微调
  • 个人偏好
  • 非关键优化

标记方式：

📝 可选：变量名 `data` 可以更具体一些，比如 `userList`。

编写有效的审查意见​

具体且可操作

不好的示例：

"这段代码有问题。"

好的示例：

"这里的递归实现对于深层嵌套的数据可能会导致栈溢出。建议改为迭代实现或使用尾递归优化。参考实现：\njava\n// 迭代实现\npublic void processNode(Node root) {\n Stack<Node> stack = new Stack<>();\n stack.push(root);\n \n while (!stack.isEmpty()) {\n Node node = stack.pop();\n process(node);\n \n if (node.right != null) stack.push(node.right);\n if (node.left != null) stack.push(node.left);\n }\n}\n"

解释原因

不好的示例：

"不要用魔法数字。"

好的示例：

"这里的 86400 是一个魔法数字，表示一天的秒数。建议提取为命名常量，提高代码可读性：\njava\nprivate static final int SECONDS_PER_DAY = 86400;\n\n// 使用\nint days = totalSeconds / SECONDS_PER_DAY;\n"

提供替代方案

当指出问题时，尽量提供可行的替代方案：

"当前的异常处理只是打印了日志，建议根据异常类型采取不同的处理策略：\n- 网络异常：可以重试\n- 业务异常：返回友好的错误信息\n- 系统异常：记录详细日志并告警"

讨论与修改​

响应审查意见​

作为代码作者

当收到审查意见时：

  1. 保持开放心态：不要将审查意见视为批评
  2. 理解意图：确保理解审查者指出的问题
  3. 及时响应：尽快修复问题或回复讨论
  4. 必要时沟通：复杂问题可以面对面讨论

回复模板：

已修复：在 commit abc123 中修改。

说明：采用了建议的方案，将循环改为 Stream API，代码确实更简洁了。

处理分歧​

当对审查意见有不同看法时：

第一步：理解对方观点

"我理解你的担忧。你是担心这个实现在高并发场景下的性能问题，对吗？"

第二步：解释你的考虑

"我选择这个方案是因为它在当前场景下更简单，而且我们的并发量并不高。不过你的担忧是有道理的。"

第三步：寻求共识

"要不我们先按当前方案实现，同时添加一个 TODO 注释，如果后续性能确实成为问题，我们再优化？"

升级处理

如果双方无法达成一致：

  • 引入第三方（如技术负责人）仲裁
  • 记录决策理由
  • 优先保证代码能够合并，避免长期阻塞

批准与合并​

批准标准​

可以批准的情况：

  • 所有阻塞性问题已解决
  • 建议性问题已处理或有合理解释
  • 代码符合团队规范
  • 测试通过

不应该批准的情况：

  • 存在未解决的阻塞性问题
  • 关键测试未通过
  • 存在已知的安全漏洞
  • 破坏了向后兼容性但未评估影响

合并策略​

Squash 合并：

适合功能开发，将多个提交压缩为一个：

feat(auth): 添加用户登录功能

- 实现登录 API
- 添加密码加密
- 集成 Redis 会话

普通合并：

适合保留历史记录的场景，如长期分支合并。

Rebase 合并：

保持线性历史，适合个人分支合并到主分支。

审查效率优化​

时间分配​

固定审查时间

建议每天安排固定的时间进行代码审查：

  • 上午：10:00-11:00
  • 下午：14:00-15:00
  • 避免：编码高峰期、会议前

及时处理

  • 收到审查请求后 2 小时内响应
  • 简单 PR 当天完成审查
  • 复杂 PR 分阶段审查

工具辅助​

自动化检查

将可以自动化的检查交给工具：

  • 代码格式（Prettier、Black）
  • 静态分析（ESLint、SonarQube）
  • 测试覆盖率
  • 安全扫描

IDE 集成

使用 IDE 插件提升审查效率：

  • GitHub Pull Requests 插件
  • GitLens
  • CodeStream

团队实践建议​

建立审查规范​

团队约定：

  • 最少审查人数
  • 响应时间要求
  • 检查清单
  • 意见分级标准

文档化

将审查规范文档化，方便新成员快速了解：

# 代码审查规范

## 审查人数
- 关键代码：2 人审查
- 一般代码：1 人审查

## 响应时间
- 收到请求后 24 小时内响应
- 紧急修复 2 小时内响应

## 审查清单
参见：docs/code-review-checklist.md

度量与改进​

关键指标：

  • 平均审查时间
  • 发现问题数量
  • 返工率
  • 审查参与度

定期回顾

每月进行一次审查流程回顾：

  • 流程中存在的问题
  • 改进建议
  • 最佳实践分享

通过持续优化，让代码审查成为提升团队效率的助力，而不是负担。