mcpskills.net
技能MCP智能体提示词
mcpskills.net — A curated directory of AI agent Skills and MCP servers
TermsPrivacy
← 返回技能
Development

代码审查与质量

多维度代码审查,适用于合并任何变更之前。审查涵盖五个维度:正确性、可读性、架构、安全性和性能。

作者:Addy Osmani仓库 →来源 →

多维度代码审查与质量门禁。每个变更在合并前都需经过审查。

使用场景

  • 合并任何 PR 或变更之前
  • 完成功能实现之后
  • 当其他代理或模型生成了需要评估的代码时
  • 重构现有代码时
  • 修复任何 Bug 之后

五维度审查

每个审查都从以下五个维度评估代码:

1. 正确性

代码是否实现了它声称的功能?

  • 是否符合规范或任务要求?
  • 边界情况是否处理(空值、空列表、边界值)?
  • 错误路径是否处理(不仅仅是正常流程)?
  • 是否通过所有测试?

2. 可读性与简洁性

其他工程师能否在没有作者解释的情况下理解这段代码?

  • 命名是否描述性且符合项目约定?
  • 控制流是否直观?
  • 代码组织是否符合逻辑?
  • 能否用更少的行数实现?

3. 架构

变更是否符合系统设计?

  • 是遵循现有模式还是引入新模式?
  • 是否保持了清晰的模块边界?
  • 依赖方向是否正确?

4. 安全性

变更是否引入了漏洞?

  • 用户输入是否验证和清理?
  • 密钥是否未泄露到代码、日志和版本控制中?
  • 是否在需要的地方检查了认证/授权?
  • SQL 查询是否参数化?

5. 性能

变更是否引入了性能问题?

  • 是否有 N+1 查询模式?
  • 是否有无界循环或不受限的数据获取?
  • UI 组件中是否有不必要的重渲染?
  • 列表端点是否缺少分页?

变更规模

控制变更规模在以下范围内:

~100 行变更   → 良好,可以一次审查完毕
~300 行变更   → 可接受,如果是单一逻辑变更
~1000 行变更  → 过大,需要拆分

拆分策略:

| 策略 | 方法 | 适用场景 | |------|------|----------| | 栈式 | 提交小变更,基于此开始下一个 | 顺序依赖 | | 按文件分组 | 为需要不同审查者的文件组分别变更 | 横切关注点 | | 水平拆分 | 先创建共享代码/桩,再创建消费者 | 分层架构 | | 垂直拆分 | 拆分为更小的全栈切片 | 功能开发 |

变更描述

每个变更都需要一个在版本控制历史中独立可读的描述。

第一行: 简短、祈使句、独立成句。"删除 FizzBuzz RPC"而非"正在删除 FizzBuzz RPC"。

正文: 变更内容和原因。包含代码本身中不可见的上下文、决策和推理。

审查流程

第 1 步:了解上下文

在查看代码之前,先了解意图:

  • 这个变更要实现什么目标?
  • 它实现了什么规范或任务?
  • 预期的行为变化是什么?

第 2 步:先审查测试

测试揭示了意图和覆盖率:

  • 是否有针对该变更的测试?
  • 测试的是行为还是实现细节?
  • 边界情况是否覆盖?

第 3 步:审查实现

带着五个维度的视角逐一检查代码:

对每个已修改的文件:

  1. 正确性:这段代码是否符合测试的预期?
  2. 可读性:我能否在没有帮助的情况下理解?
  3. 架构:是否符合系统设计?
  4. 安全性:是否有漏洞?
  5. 性能:是否有瓶颈?

第 4 步:分类标注

为每条评论标注严重程度:

| 前缀 | 含义 | 作者操作 | |------|------|----------| | (无前缀) | 必须修改 | 合并前必须处理 | | Critical: | 阻止合并 | 安全漏洞、数据丢失、功能损坏 | | Nit: | 轻微、可选 | 作者可忽略——格式、风格偏好 | | Optional: / Consider: | 建议 | 值得考虑但非必需 | | FYI | 仅供参考 | 无需操作 |

第 5 步:核实验证

检查作者的验证过程:

  • 运行了哪些测试?
  • 构建是否通过?
  • 是否手动测试了变更?
  • UI 变更是否有截图?

审查清单

## 审查:[PR/变更标题]

### 上下文
- [ ] 我理解这个变更的内容和原因

### 正确性
- [ ] 变更符合规范/任务要求
- [ ] 边界情况已处理
- [ ] 错误路径已处理
- [ ] 测试充分覆盖变更

### 可读性
- [ ] 命名清晰且一致
- [ ] 逻辑直观
- [ ] 无不必要的复杂性

### 架构
- [ ] 遵循现有模式
- [ ] 无不必要的耦合或依赖
- [ ] 抽象层级适当

### 安全性
- [ ] 代码中无密钥
- [ ] 边界处输入已验证
- [ ] 无注入漏洞
- [ ] 认证检查到位

### 性能
- [ ] 无 N+1 模式
- [ ] 无无界操作
- [ ] 列表端点有分页

### 验证
- [ ] 测试通过
- [ ] 构建成功
- [ ] 已手动验证(如适用)

### 结论
- [ ] **通过** —— 可以合并
- [ ] **需要修改** —— 必须处理问题

审查速度

  • 在一个工作日内响应 —— 这是上限而非目标
  • 理想节奏: 审查请求到达后尽快响应
  • 优先快速响应 而非快速最终批准
  • 大规模变更: 要求作者拆分

处理分歧

在解决审查争议时,按以下优先级处理:

  1. 技术事实和数据 优先于观点和偏好
  2. 代码风格指南 是风格事项的绝对权威
  3. 软件设计 必须基于工程原则评估
  4. 代码库一致性 在不降低整体质量的前提下是可以接受的

验证

审查完成后:

  • [ ] 所有关键问题已解决
  • [ ] 所有必须修改的事项已解决或明确推迟
  • [ ] 测试通过
  • [ ] 构建成功
  • [ ] 验证过程已记录