Code Review 是保证代码质量和知识共享的关键实践。本文探讨高效代码审查的方法和工具。
Code Review 概述
为什么需要 Code Review
Code Review 价值：
┌─────────────────────────────────────────────────────┐
│ │
│ 质量保证 │
│ ├── 发现 Bug 和逻辑错误 │
│ ├── 确保代码符合规范 │
│ ├── 识别安全漏洞 │
│ └── 提高代码可维护性 │
│ │
│ 知识共享 │
│ ├── 团队成员了解代码库 │
│ ├── 学习最佳实践 │
│ ├── 传承技术决策 │
│ └── 培养新人 │
│ │
│ 协作提升 │
│ ├── 统一编码风格 │
│ ├── 建立信任 │
│ ├── 促进沟通 │
│ └── 集体代码所有权 │
│ │
└─────────────────────────────────────────────────────┘| 目标 | 关注点 |
|---|---|
| 正确性 | 逻辑、边界条件、错误处理 |
| 可读性 | 命名、结构、注释 |
| 可维护性 | 模块化、复杂度、测试 |
| 性能 | 算法效率、资源使用 |
创建优质 PR
PR 规范
## PR 模板
### 变更描述
<!-- 清晰描述这个 PR 做了什么 -->
### 变更类型
- [ ] Bug 修复
- [ ] 新功能
- [ ] 重构
- [ ] 文档更新
- [ ] 测试
### 相关 Issue
<!-- 链接相关的 Issue -->
Closes #123
### 测试说明
<!-- 描述如何测试这些变更 -->
### 截图（如适用）
<!-- UI 变更请附上截图 -->
### 检查清单
- [ ] 代码符合项目规范
- [ ] 已添加必要的测试
- [ ] 文档已更新
- [ ] 本地测试通过保持 PR 小而专注
// ❌ 大型 PR - 难以审查
// PR #1: 重构用户模块 + 添加新功能 + 修复 Bug + 更新依赖
// 涉及 50+ 文件，2000+ 行变更
// ✅ 拆分为多个小 PR
// PR #1: 重构用户数据模型（5 文件，100 行）
// PR #2: 添加用户搜索功能（8 文件，200 行）
// PR #3: 修复用户登录 Bug（2 文件，30 行）
// PR #4: 升级认证库版本（3 文件，50 行）
// 理想的 PR 大小
const idealPRSize = {
files: '< 10',
linesChanged: '< 400',
reviewTime: '< 30 minutes',
focusedOn: 'Single concern',
};有意义的 Commit 信息
# Commit 信息规范
# <type>(<scope>): <subject>
#
# <body>
#
# <footer>
# 示例
git commit -m "feat(auth): add OAuth2 login support
- Implement Google OAuth2 provider
- Add token refresh mechanism
- Update user model with provider field
Closes #456"
# 类型说明
# feat: 新功能
# fix: Bug 修复
# docs: 文档更新
# style: 格式调整（不影响代码逻辑）
# refactor: 重构
# test: 测试相关
# chore: 构建/工具变更审查技巧
审查清单
// 代码审查检查点
const reviewChecklist = {
// 1. 正确性
correctness: [
'逻辑是否正确？',
'边界条件是否处理？',
'错误处理是否完善？',
'是否有潜在的空指针？',
],
// 2. 设计
design: [
'是否符合现有架构？',
'是否过度设计？',
'是否需要重构？',
'依赖关系是否合理？',
],
// 3. 可读性
readability: [
'命名是否清晰？',
'代码是否易于理解？',
'注释是否必要且准确？',
'函数是否太长？',
],
// 4. 测试
testing: [
'是否有足够的测试？',
'测试是否覆盖边界情况？',
'测试是否易于维护？',
],
// 5. 安全
security: [
'是否有注入风险？',
'敏感数据是否安全处理？',
'权限检查是否到位？',
],
// 6. 性能
performance: [
'是否有性能问题？',
'N+1 查询？',
'内存泄漏风险？',
],
};审查优先级
// 审查重点分层
const reviewPriorities = {
// 高优先级 - 必须修复
critical: [
'Bug 和逻辑错误',
'安全漏洞',
'数据丢失风险',
'严重性能问题',
],
// 中优先级 - 建议修复
major: [
'设计问题',
'代码重复',
'缺少错误处理',
'测试覆盖不足',
],
// 低优先级 - 可选改进
minor: [
'代码风格',
'命名建议',
'文档完善',
'微优化',
],
};提供有效反馈
建设性评论
## 评论示例
### ❌ 不好的评论
"这段代码很糟糕"
"为什么要这样写？"
"错了"
### ✅ 好的评论
**提出问题而非命令**
"这里使用 `map` 代替 `forEach` 会更清晰吗？"
**解释原因**
"建议将这个函数拆分成更小的函数。
原因：当前函数有 60 行，难以理解和测试。"
**提供示例**
"考虑使用早期返回模式：
```typescript
// 当前
if (user) {
// 50 行逻辑
}
// 建议
if (!user) return;
// 50 行逻辑
```"
**区分优先级**
"[必须] 这里需要处理空值情况"
"[建议] 这个变量名可以更具描述性"
"[讨论] 这种方式和 X 方式哪个更好？"评论模板
// 评论类型标签
const commentLabels = {
'[必须]': '必须修复才能合并',
'[建议]': '强烈建议但非阻塞',
'[提问]': '需要澄清的问题',
'[讨论]': '开放性讨论',
'[赞]': '代码亮点',
'[nit]': '细节优化，可忽略',
};
// 示例评论
const exampleComments = [
'[必须] 这个 API 调用缺少错误处理，可能导致未捕获的异常',
'[建议] 考虑使用 `const` 替代 `let`，因为这个变量不会被重新赋值',
'[提问] 这里选择同步而非异步的原因是什么？',
'[讨论] 对于这种场景，使用 Redux 还是 Context 更合适？',
'[赞] 这个抽象设计得很优雅，复用性很好！',
'[nit] 这里多了一个空行',
];自动化工具
CI/CD 集成
# .github/workflows/pr-check.yml
name: PR Check
on:
pull_request:
branches: [main, develop]
jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v2
- uses: actions/setup-node@v4
with:
node-version: '20'
cache: 'pnpm'
- run: pnpm install
- run: pnpm lint
- run: pnpm typecheck
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v2
- uses: actions/setup-node@v4
with:
node-version: '20'
cache: 'pnpm'
- run: pnpm install
- run: pnpm test --coverage
- name: Upload coverage
uses: codecov/codecov-action@v3
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: pnpm/action-setup@v2
- uses: actions/setup-node@v4
with:
node-version: '20'
cache: 'pnpm'
- run: pnpm install
- run: pnpm build自动化检查工具
# PR 自动化检查配置
# .github/workflows/auto-review.yml
name: Auto Review
on:
pull_request:
types: [opened, synchronize]
jobs:
size-check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Check PR size
uses: codelytv/pr-size-labeler@v1
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
xs_label: 'size/XS'
xs_max_size: 10
s_label: 'size/S'
s_max_size: 100
m_label: 'size/M'
m_max_size: 500
l_label: 'size/L'
l_max_size: 1000
xl_label: 'size/XL'
danger:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Danger
uses: danger/danger-js@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}Danger.js 配置
// dangerfile.js
import { danger, warn, fail, message } from 'danger';
// PR 大小检查
const bigPRThreshold = 500;
if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) {
warn('PR 太大了，考虑拆分成更小的 PR');
}
// 检查 PR 描述
if (danger.github.pr.body.length < 50) {
fail('请提供更详细的 PR 描述');
}
// 检查测试文件
const hasTestChanges = danger.git.modified_files.some(
(file) => file.includes('.test.') || file.includes('.spec.')
);
const hasSourceChanges = danger.git.modified_files.some(
(file) => file.includes('src/') && !file.includes('.test.')
);
if (hasSourceChanges && !hasTestChanges) {
warn('源代码有变更但没有更新测试');
}
// 检查是否修改了敏感文件
const sensitiveFiles = ['.env', 'package-lock.json', 'yarn.lock'];
const modifiedSensitive = danger.git.modified_files.filter((file) =>
sensitiveFiles.some((s) => file.includes(s))
);
if (modifiedSensitive.length > 0) {
warn(`修改了敏感文件: ${modifiedSensitive.join(', ')}`);
}
// 庆祝首次贡献
if (danger.github.pr.author_association === 'FIRST_TIME_CONTRIBUTOR') {
message('欢迎首次贡献！🎉');
}最佳实践总结
Code Review 最佳实践：
┌─────────────────────────────────────────────────────┐
│ │
│ 作为作者 │
│ ├── 保持 PR 小而专注 │
│ ├── 提供清晰的描述和上下文 │
│ ├── 自己先审查一遍 │
│ └── 及时响应反馈 │
│ │
│ 作为审查者 │
│ ├── 及时审查（24小时内） │
│ ├── 提供建设性反馈 │
│ ├── 区分优先级 │
│ └── 肯定好的实践 │
│ │
│ 团队规范 │
│ ├── 建立审查清单 │
│ ├── 使用自动化工具 │
│ ├── 定期回顾流程 │
│ └── 培养审查文化 │
│ │
└─────────────────────────────────────────────────────┘| 场景 | 建议 |
|---|---|
| 大型变更 | 拆分成多个 PR |
| 紧急修复 | 简化流程但仍需审查 |
| 新人代码 | 提供详细指导 |
| 复杂逻辑 | 安排讨论会议 |
Code Review 不是找茬，是团队共同成长。