Code Review 最佳实践：作为评审者和被评审者
Code Review 做了两年，从被骂到被认可，记录一些经验。
为什么要做 Code Review
| 好处 | 说明 |
|---|---|
| 发现 Bug | 多双眼睛多份保障 |
| 知识共享 | 团队成员互相学习 |
| 统一风格 | 保持代码一致性 |
| 降低风险 | 避免单点依赖 |
作为被评审者
MR 要小
一个 MR 只做一件事：
✅ 修复用户登录验证的 Bug
❌ 修复登录 Bug + 重构用户模块 + 更新依赖小 MR 容易审，反馈快，问题少。
写好描述
## 变更内容
- 修复了用户登录时 token 过期无法刷新的问题
## 原因
- 之前的实现在 token 刷新时没有正确处理并发请求
## 测试
- 单元测试：新增 3 个测试用例
- 手动测试：模拟并发刷新场景
## 截图
（如有 UI 变化，附上截图）不要只写“修复 Bug”，让评审者猜。
自己先审一遍
提交前检查：
| 检查项 | 说明 |
|---|---|
| 编译/构建 | 能跑起来吗 |
| 测试 | 新代码有测试吗 |
| 注释 | 复杂逻辑有注释吗 |
| 格式 | 符合团队规范吗 |
| 调试代码 | console.log 删了吗 |
面对评论
不要把评论当人身攻击：
评审者：这个函数命名不清晰两种回应：
❌ 这名字挺好的啊，你看不懂吗
✅ 确实容易误解，改成 validateUserToken 怎么样评审者指出问题是帮你改进，不是找茬。
作为评审者
控制评审时间
| MR 大小 | 建议时间 |
|---|---|
| < 100 行 | 1 天内 |
| 100-300 行 | 2 天内 |
| > 300 行 | 建议拆分 |
延迟评审会影响团队效率。
关注重点
按优先级：
- 正确性：逻辑对吗？有 Bug 吗？
- 安全性：有漏洞吗？
- 性能：有性能问题吗？
- 可读性：别人能看懂吗？
- 风格：符合规范吗？
不要纠结 2 空格还是 4 空格，交给格式化工具。
评论要有建设性
❌ 这代码写得烂
✅ 这里用 Map 替代 Object 会更好，因为...
❌ 不对
✅ 考虑一下边界情况：如果 user 为 null 会怎样？
❌ 重写
✅ 这个函数职责太多，建议拆成 validate 和 transform告诉对方“为什么”和“怎么改”。
区分必须和建议
[P2] 必须修复：这里有 SQL 注入风险
[Nit] 建议：变量名可以更有语义，不强制让被评审者知道哪些必须改，哪些可以讨论。
常见反模式
评审者
| 反模式 | 问题 | 改进 |
|---|---|---|
| 全部通过 | 没有意义 | 至少看一遍 |
| 过于严格 | 打击积极性 | 区分必须和建议 |
| 主观偏好强推 | 引发冲突 | 尊重团队规范 |
| 拖延评审 | 阻塞开发 | 设置提醒 |
被评审者
| 反模式 | 问题 | 改进 |
|---|---|---|
| 巨型 MR | 无法审 | 拆分成小 MR |
| 没有描述 | 猜意图 | 写清楚变更原因 |
| 拒绝修改 | 无法进步 | 虚心接受建议 |
| 立即催审 | 压力大 | 给评审者合理时间 |
团队规范
我们团队的 Code Review 规范：
## MR 规范
1. 单个 MR 不超过 300 行
2. 必须有描述，说明变更内容和原因
3. 新代码必须有测试
4. CI 通过才能请求评审
## 评审规范
1. 工作日 24 小时内响应
2. 必须评论，不能只点赞
3. 区分 [P2] 必须修复 和 [Nit] 建议
4. 有争议拉会议讨论，不在评论里吵架
## 通过标准
- 所有 [P2] 问题已解决
- 测试通过
- 至少一个 Approve数据驱动
我们追踪的指标：
| 指标 | 目标 | 说明 |
|---|---|---|
| 评审响应时间 | < 4 小时 | 从请求到首次评论 |
| MR 大小 | < 200 行 | 平均值 |
| 评审参与率 | 100% | 每个 MR 都有人审 |
| 线上 Bug | 下降 | Review 带来的质量提升 |
总结
Code Review 的核心是沟通，不是挑刺。
好的 Code Review 文化：
- 评审者帮被评审者成长
- 被评审者从反馈中学习
- 团队代码质量共同提升
关键是心态：把 Review 当成学习机会，不是考试。