代码审查与质量
多维度代码审查,适用于合并任何变更之前。审查涵盖五个维度:正确性、可读性、架构、安全性和性能。
多维度代码审查与质量门禁。每个变更在合并前都需经过审查。
每个审查都从以下五个维度评估代码:
代码是否实现了它声称的功能?
其他工程师能否在没有作者解释的情况下理解这段代码?
变更是否符合系统设计?
变更是否引入了漏洞?
变更是否引入了性能问题?
控制变更规模在以下范围内:
~100 行变更 → 良好,可以一次审查完毕
~300 行变更 → 可接受,如果是单一逻辑变更
~1000 行变更 → 过大,需要拆分
拆分策略:
| 策略 | 方法 | 适用场景 | |------|------|----------| | 栈式 | 提交小变更,基于此开始下一个 | 顺序依赖 | | 按文件分组 | 为需要不同审查者的文件组分别变更 | 横切关注点 | | 水平拆分 | 先创建共享代码/桩,再创建消费者 | 分层架构 | | 垂直拆分 | 拆分为更小的全栈切片 | 功能开发 |
每个变更都需要一个在版本控制历史中独立可读的描述。
第一行: 简短、祈使句、独立成句。"删除 FizzBuzz RPC"而非"正在删除 FizzBuzz RPC"。
正文: 变更内容和原因。包含代码本身中不可见的上下文、决策和推理。
在查看代码之前,先了解意图:
测试揭示了意图和覆盖率:
带着五个维度的视角逐一检查代码:
对每个已修改的文件:
为每条评论标注严重程度:
| 前缀 | 含义 | 作者操作 | |------|------|----------| | (无前缀) | 必须修改 | 合并前必须处理 | | Critical: | 阻止合并 | 安全漏洞、数据丢失、功能损坏 | | Nit: | 轻微、可选 | 作者可忽略——格式、风格偏好 | | Optional: / Consider: | 建议 | 值得考虑但非必需 | | FYI | 仅供参考 | 无需操作 |
检查作者的验证过程:
## 审查:[PR/变更标题]
### 上下文
- [ ] 我理解这个变更的内容和原因
### 正确性
- [ ] 变更符合规范/任务要求
- [ ] 边界情况已处理
- [ ] 错误路径已处理
- [ ] 测试充分覆盖变更
### 可读性
- [ ] 命名清晰且一致
- [ ] 逻辑直观
- [ ] 无不必要的复杂性
### 架构
- [ ] 遵循现有模式
- [ ] 无不必要的耦合或依赖
- [ ] 抽象层级适当
### 安全性
- [ ] 代码中无密钥
- [ ] 边界处输入已验证
- [ ] 无注入漏洞
- [ ] 认证检查到位
### 性能
- [ ] 无 N+1 模式
- [ ] 无无界操作
- [ ] 列表端点有分页
### 验证
- [ ] 测试通过
- [ ] 构建成功
- [ ] 已手动验证(如适用)
### 结论
- [ ] **通过** —— 可以合并
- [ ] **需要修改** —— 必须处理问题
在解决审查争议时,按以下优先级处理:
审查完成后: