Code Review

核心问题：如何做好 Code Review？Review 的标准是什么？如何让 Code Review 成为团队文化？

1. Code Review 的价值

价值	说明
代码质量	发现 Bug、设计缺陷、代码坏味道
知识共享	团队成员了解彼此的代码
一致性	统一代码风格和设计模式
成长	新人从 Review 中学习，资深工程师通过 Review 教学

2. Code Review Checklist

## Code Review 检查清单

### 正确性
□ 代码逻辑是否正确？
□ 边界条件是否处理？
□ 异常处理是否完善？
□ 并发安全是否考虑？

### 设计
□ 是否遵循 SOLID 原则？
□ 是否有代码坏味道？
□ 抽象层次是否合理？
□ 命名是否清晰？

### 可维护性
□ 代码是否易于理解？
□ 是否有适当的注释？
□ 是否有单元测试？
□ 测试覆盖率是否足够？

### 性能
□ 是否有 N+1 查询？
□ 是否有内存泄漏风险？
□ 是否有不必要的循环？

### 安全
□ 是否有 SQL 注入风险？
□ 是否有 XSS 风险？
□ 敏感数据是否加密？

### 变更风险
□ 是否改变公开 API、数据格式或异常语义？
□ 是否存在竞态条件、重复执行或事务边界变化？
□ 数据变更是否兼容新旧版本并存，回滚是否安全？
□ 关键失败路径是否有测试，是否需要灰度或监控？

3. Review 评论规范

// 好的 Review 评论：具体、可操作、解释原因

// 差：模糊的评论
// "这段代码不好"

// 好：具体的评论（说明触发条件与依据）
// "循环中反复向同一字符串追加内容时，重复拼接可能反复复制已有字符，
//  累计成本可能接近二次增长。建议改用 StringBuilder，并用基准测试
//  确认这条路径是否值得优化。"

// 好：指出问题并给出解决方案（说明适用条件）
// "这里可能存在 N+1 查询问题。可使用 @EntityGraph 或 JOIN FETCH，
//  但需结合关联类型、查询方式与分页需求选择；集合关联配合分页时，
//  JOIN FETCH 可能导致内存分页或触发异常，可先分页查询主实体，再批量加载关联数据。"

// 好：认可好的代码
// "这个策略模式的使用很优雅，后续新增支付方式只需要添加新的实现类 👍"

发现 Review 问题后，按严重程度分级处理：

级别	判定	处理方式
阻断问题	正确性、安全性、数据完整性存在实质风险	修复或明确解决方案前不应合并
建议性问题	可维护性、风格或非关键设计改进	由作者与 Reviewer 协商是否本次处理
待验证问题	证据不足，尚不能确认是缺陷	补充测试、测量或代码依据，不把推测当缺陷

作者可以解释设计取舍，但不能仅凭口头解释替代必要的验证。

4. Review 工具与流程

// GitHub Pull Request 流程
// 1. 创建分支，开发功能
// 2. 提交 PR，填写描述
// 3. 自动触发 CI（构建 + 测试 + 代码扫描）
// 4. 指定 Reviewer 进行 Review
// 5. 根据反馈修改代码
// 6. 满足团队规定的审批要求后合并（例如至少 1 人 Approve）；高风险变更按需增加领域负责人审批

// PR 描述模板
/**
 * ## 变更说明
 * 简述本次变更的内容和目的
 * 
 * ## 变更类型
 * - [ ] 新功能
 * - [ ] Bug 修复
 * - [ ] 重构
 * - [ ] 文档更新
 * 
 * ## 测试
 * - [ ] 单元测试已添加/更新
 * - [ ] 本地测试通过
 * 
 * ## 影响范围
 * 描述本次变更影响的功能模块
 * 
 * ## 截图（UI 变更时）
 */

5. Review 文化建设

实践	说明
小 PR	每次 PR 控制在 200-400 行，便于 Review
及时 Review	收到 PR 后 24 小时内完成 Review
正面反馈	认可好的代码，不只是挑问题
对事不对人	评论针对代码，不针对人
自我 Review	提交 PR 前先自己 Review 一遍

上表中的行数与时限是团队约定的默认值，不是通用硬性标准。审查投入应与变更风险匹配：普通、局部且易验证的变更走常规流程；涉及数据迁移、权限控制、并发正确性、公共接口或不可逆操作的变更，应提高审查要求，必要时增加领域负责人审批、专项测试或发布保护。

核心原则：Code Review 不是找茬，而是团队共同提升代码质量的过程。好的 Review 文化应该是"我们一起让代码更好"，而不是"我来挑你的毛病"。