Code Review 实践指南

发布于 2024年6月10日

什么是 Code Review？

Code Review 就是团队成员互相看代码、给反馈的过程。研究表明，Code Review 可以找到约 65% 的错误。

为什么要做 Code Review？

1. 知识共享

新人能从老手那学到经验
了解项目的不同模块是怎么实现的
避免”只有某个人懂某块代码”的情况

// ❌ 新人可能会这样写
function getUserData(userId) {
  return fetch(`/api/users/${userId}`)
    .then(res => res.json())
    .catch(err => console.log(err))
}

// ✅ Review 后的改进版本
async function getUserData(userId) {
  try {
    const response = await fetch(`/api/users/${userId}`)
    if (!response.ok) {
      throw new Error(`HTTP error! status: ${response.status}`)
    }
    return await response.json()
  } catch (error) {
    logger.error('Failed to fetch user data', { userId, error })
    throw error
  }
}

2. 提高代码质量

多一双眼睛能发现更多问题
知道别人会看，写代码时会更用心
保持团队代码风格统一

3. 发现 Bug

新的视角更容易发现问题
边界情况、异常处理等容易被忽略的地方
性能问题、安全隐患

4. 团队协作

增进技术交流
培养共同责任感
形成良好的工程文化

什么时候做 Review？

提交前自查

提交代码前先自己过一遍：

只提交相关的改动
删掉调试代码和注释
加上必要的测试
更新相关文档

# 提交前看看改了啥
git diff --staged

PR Review（主要场景）

功能开发完、自测通过后，创建 Pull Request：

## PR 描述模板

### 改了什么
简单说明主要改动

### 相关 Issue
Closes #123

### 测试情况
- [ ] 单元测试通过
- [ ] 手动测试通过
- [ ] 测试场景：xxx

### 需要注意的点
列出需要 Reviewer 特别关注的地方

合并前最后检查

至少 1 人 approve
讨论都解决了
CI 都过了
没有冲突

使用什么工具？

GitHub / GitLab（推荐）

最常用的 Code Review 平台：

行级别的评论
代码建议功能
CI/CD 集成
Review 状态管理

基本流程：

# 1. 创建分支
git checkout -b feature/xxx

# 2. 开发、提交
git commit -m "feat: xxx"

# 3. 推送、创建 PR
git push origin feature/xxx

# 4. Review、修改、合并

自动化工具

让机器做能自动化的事：

# .github/workflows/pr-check.yml
name: PR Check
on: [pull_request]

jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Lint
        run: npm run lint
      - name: Test
        run: npm test

机器负责：

代码格式（Prettier, ESLint）
跑测试
测试覆盖率
安全扫描

人负责：

业务逻辑对不对
架构设计合不合理
代码好不好读、好不好维护

Code Review 怎么做？

作为提交者

确保代码能跑

npm test && npm run lint && npm run build

写清楚 PR 描述
- 为什么做这个改动（比做了什么更重要）
- 贴上相关文档或 Issue
- 标注需要特别注意的地方
先自己 Review 一遍
- 看看有没有无关的改动
- 删掉临时代码和 TODO

作为 Reviewer

第一遍：看整体

理解为什么要改这些
整体架构合不合理
改动范围是否合适

第二遍：看细节

功能逻辑：

逻辑正确吗？
边界情况处理了吗？
错误处理完善吗？

代码质量：

好读吗？
有没有过度设计？
有没有重复代码？

性能和安全：

有性能问题吗？
有安全隐患吗？

怎么给反馈？

❌ 不好的评论：
"这里不对"

✅ 好的评论：
"这里用 map() 会创建新数组但没用到返回值，
建议用 forEach() 或 for...of，性能更好：

\`\`\`javascript
// 建议改成
items.forEach(item => processItem(item))
\`\`\`
"

原则：

🎯 对事不对人
💡 说清楚为什么
🤔 用疑问的语气：“是不是可以…”
👍 好的地方也要表扬

反馈的优先级

用标签区分轻重：

🔴 MUST：必须改
🟡 SHOULD：强烈建议改
🟢 NICE：可以改，锦上添花
💭 QUESTION：有疑问，讨论一下

🔴 MUST: 这里会空指针，必须加 null 检查

🟡 SHOULD: 这个函数 100 多行了，建议拆分

🟢 NICE: 可以用 Optional 让代码更优雅

💭 QUESTION: 为啥用这个方案？考虑过 XXX 吗？

最佳实践

1. 控制 PR 大小

每次不超过 400 行（Cisco 研究数据）
太大的改动拆成多个 PR
小的改动更容易发现问题

# 看看 PR 有多大
git diff --stat main...feature-branch

2. 及时 Review

24 小时内开始 Review
避免阻塞别人的工作
趁着还记得，更好讨论

3. 用 Checklist

常用检查清单：

常见问题

❌ 橡皮图章

不看代码就点 Approve。

解决：至少要求留一条评论才能 approve

❌ 纠结细枝末节

花半天争论变量名，却忽略了严重的性能问题。

// 争论这个命名
const userData = getUserData()

// 却没发现这个 N+1 查询问题
users.forEach(user => {
  const orders = db.query(`SELECT * FROM orders WHERE user_id = ${user.id}`)
})

解决：用工具处理格式问题，人关注重要问题

❌ 过度设计

要求添加当前用不到的功能”万一以后要用呢”。

记住：YAGNI（You Aren’t Gonna Need It）

❌ 拖延

PR 堆了好几天没人看。

解决：把 Review 当成日常工作，设置提醒

❌ 情绪化评论

❌ "这什么垃圾代码？"
❌ "你连这个都不会？"

✅ "这里可能有性能问题，讨论一下更好的方案？"
✅ "考虑过用 XXX 模式吗？可能更合适"

流程图

graph TD
    A[开发完成] --> B{自我审查}
    B -->|通过| C[提交 PR]
    B -->|不通过| D[修改代码]
    D --> B
    C --> E{CI 检查}
    E -->|失败| D
    E -->|通过| F{同行审查}
    F -->|需要修改| G[讨论并修改]
    F -->|批准| H[合并代码]
    G --> F
    H --> I[结束]

总结

记住这几点

✅ 控制规模（不超过 400 行）
✅ 及时响应（24 小时内）
✅ 自动化工具（格式、测试交给机器）
✅ 建设性反馈（对事不对人）
✅ 持续改进（定期回顾）

避免这些坑

❌ 敷衍了事
❌ 纠结小事
❌ 拖延不看
❌ 情绪化评论

最重要的：Code Review 不是找茬，是一起把代码写得更好。