跳到主要内容

如何做好 Code Review

问题​

你们团队是怎么做 Code Review 的？你觉得好的 CR 应该关注什么？

面试速答版

你们团队是怎么做 Code Review 的？ CR 不是「找茬」，而是团队共同对代码质量负责的机制：

  • 流程：所有 PR 必须至少 1 人 Review 才能合，核心模块要 2 人
  • 工具：GitLab/GitHub Review，配 lint/test 自动检查
  • 响应时效：24 小时内必须给反馈，不阻塞别人
  • 文化：对事不对人，作者解释思路，Reviewer 提建议而非命令

你觉得好的 CR 应该关注什么？ 按层次分清主次，不要把时间浪费在低优先级问题上：

  • 正确性：逻辑、边界条件、并发/异步问题——最高优先级
  • 安全性：XSS、SQL 注入、敏感信息泄漏、依赖漏洞
  • 可读性：命名、函数粒度、复杂度、是否需要注释
  • 性能：明显的 N+1、不必要的重渲染、大对象传递
  • 可维护性：是否符合架构、有无重复代码、测试覆盖
  • 风格问题：交给 Lint/Prettier，CR 不要纠结

回答思路​

1. 为什么要做 Code Review​

目的说明
发现缺陷另一双眼睛比自测更容易发现问题
知识共享团队成员互相学习代码风格和技术方案
统一规范保持代码库风格一致
提升质量从设计层面提出改进意见
减少 Bus Factor至少两个人了解每段代码
提示

CR 不是"找茬"，而是团队一起让代码变得更好。

2. CR 关注的层次​

3. CR Checklist​

作为 Reviewer​

## Code Review 检查清单

### ✅ 功能正确性
- [ ] 逻辑完整，覆盖正常和异常路径
- [ ] 边界条件处理（空数组、null、undefined）
- [ ] 错误处理完善（try-catch、错误边界）

### ✅ 代码设计
- [ ] 函数/组件职责单一
- [ ] 没有过度设计，也没有设计不足
- [ ] 复用已有的工具函数/组件
- [ ] 新增文件的位置合理

### ✅ 代码可读性
- [ ] 变量和函数命名准确、一致
- [ ] 复杂逻辑有必要的注释
- [ ] 代码结构清晰，容易理解

### ✅ TypeScript 类型
- [ ] 类型定义完整，避免 any
- [ ] 使用合适的泛型和工具类型
- [ ] 类型导出位置合理

### ✅ 性能
- [ ] 没有不必要的重渲染
- [ ] 大数据量有分页或虚拟滚动
- [ ] 没有内存泄漏风险

### ✅ 安全
- [ ] 用户输入有验证
- [ ] 不存在 XSS 风险（如 dangerouslySetInnerHTML）
- [ ] 敏感信息不暴露在前端

作为 Author（提交者）​

PR 提交前的自检：

## PR 描述模板

### What（做了什么）
简述变更内容

### Why（为什么这么做）
背景和技术方案说明

### How（如何验证）
- [ ] 本地测试通过
- [ ] 相关单元测试已添加/更新
- [ ] CI 检查通过
- [ ] 影响范围评估

### Screenshots（截图）
如有 UI 变更，附截图

4. 好的 CR 评论示例​

❌ 不好的 CR 评论
// "这段代码有问题" ← 没说什么问题、怎么改

// "这样写不好" ← 没解释为什么、没给替代方案
✅ 好的 CR 评论
// 🔴 问题：这里直接拼接 HTML 可能产生 XSS 风险
// 建议：使用 DOMPurify 或 React 的 JSX 自动转义
// 参考：https://owasp.org/www-community/xss-filter-evasion-cheatsheet

// 💡 建议：这个函数已经有 50 行了，建议拆分为：
// 1. validateInput() - 参数校验
// 2. transformData() - 数据转换
// 3. formatOutput() - 格式化输出
// 这样每个函数职责更清晰，也更容易测试

// ❓ 疑问：这里为什么选择 useRef 而不是 useState？
// 如果有特殊原因，建议加个注释说明

5. CR 流程最佳实践​

实践说明
PR 保持小每个 PR 不超过 400 行，大需求分多次提交
及时 Review24 小时内完成 Review，不要 block 别人
最少 1 个 Approve至少一个人 Approve 才能合并
自动化兜底ESLint + 单元测试 + CI 门禁处理格式和基础问题
区分严重级别🔴 Must Fix / 🟡 Suggestion / 🟢 Nitpick
先整体后细节先看整体设计，再看代码细节

常见面试问题​

Q1: 你在 CR 中发现过什么典型问题？​

答案：

举一个具体例子：

CR 中发现的内存泄漏
// ❌ 提交的代码 —— 组件卸载后定时器没清理
function PollingComponent() {
  const [data, setData] = useState(null);
  
  useEffect(() => {
    const timer = setInterval(async () => {
      const res = await fetchData();
      setData(res); // 组件卸载后仍然 setState → 内存泄漏
    }, 5000);
    // 缺少 cleanup！
  }, []);
  
  return <div>{data}</div>;
}

// ✅ CR 后修正
function PollingComponent() {
  const [data, setData] = useState(null);
  
  useEffect(() => {
    let active = true;
    const timer = setInterval(async () => {
      const res = await fetchData();
      if (active) setData(res);
    }, 5000);
    
    return () => {
      active = false;
      clearInterval(timer);
    };
  }, []);
  
  return <div>{data}</div>;
}

Q2: 团队抵触 CR 怎么办？​

答案：

  1. 找原因：是觉得浪费时间？CR 评论太刻薄？还是 PR 太大导致 Review 困难？
  2. 降低成本：PR 保持小、自动化处理格式问题、提供 CR 模板
  3. 改善体验：培训 Review 技巧、约定评论礼仪（对事不对人）
  4. 展示价值：统计 CR 发现的 Bug 数量，让数据说话
  5. 以身作则：Lead 先做、做好，带动团队

Q3: 自动化能替代人工 CR 吗？​

答案：

不能完全替代，但可以互补：

自动化人工
代码格式 ✅设计合理性 ✅
语法错误 ✅业务逻辑正确性 ✅
简单 Bug ✅架构层面建议 ✅
安全扫描（基础）✅复杂安全问题 ✅

最佳实践：让机器做重复性检查，人聚焦在设计和逻辑上。AI Code Review 工具（如 CodeRabbit）可以进一步减少人工负担，但核心设计决策仍需人工判断。

相关链接​