| 目标 | 说明 |
|---|
| 质量保障 | 发现缺陷和设计问题 |
| 知识共享 | 团队成员互相学习 |
| 标准统一 | 确保编码规范一致 |
| 导师作用 | 帮助新人成长 |
| 原则 | 说明 |
|---|
| 对事不对人 | 评论代码而非评论人 |
| 及时反馈 | 24小时内完成审查 |
| 建设性意见 | 提出改进建议而非仅指出问题 |
| 尊重作者 | 理解作者的思考过程 |
| 检查项 | 说明 |
|---|
| 功能正确 | 代码是否实现了需求 |
| 边界条件 | 是否处理了边界情况 |
| 错误处理 | 异常是否被正确处理 |
| 并发安全 | 是否存在竞态条件 |
| 资源释放 | 资源是否正确释放 |
| 检查项 | 说明 |
|---|
| 命名规范 | 变量/函数/类命名是否清晰 |
| 代码结构 | 逻辑是否清晰易懂 |
| 注释适当 | 复杂逻辑是否有注释 |
| 函数长度 | 函数是否过长（>30行） |
| 嵌套深度 | 嵌套是否过深（>3层） |
| 检查项 | 说明 |
|---|
| 单一职责 | 函数/类是否只做一件事 |
| 重复代码 | 是否存在重复逻辑 |
| 硬编码 | 是否有魔法数字/字符串 |
| 耦合度 | 模块间是否过度耦合 |
| 可扩展性 | 新增功能是否需要大量修改 |
| 检查项 | 说明 |
|---|
| 输入验证 | 是否验证外部输入 |
| SQL注入 | 是否使用参数化查询 |
| XSS | 是否转义用户输入 |
| 敏感数据 | 是否泄露密钥/密码 |
| 权限控制 | 是否检查访问权限 |
| 检查项 | 说明 |
|---|
| N+1查询 | 是否存在循环内查询 |
| 内存泄漏 | 是否有大对象未释放 |
| 不必要计算 | 是否有可避免的计算 |
| 缓存策略 | 是否合理使用缓存 |
| 批量操作 | 是否使用批量代替循环 |
| 检查项 | 说明 |
|---|
| 测试覆盖 | 是否有对应测试 |
| 测试质量 | 测试是否真正验证行为 |
| 边界测试 | 是否测试了边界条件 |
| Mock合理 | Mock是否合理 |
1. 自我审查代码
2. 编写清晰的PR描述
3. 关联相关Issue
4. 添加必要的截图/日志
5. 指定审查者
1. 理解PR的目标和背景
2. 从整体到细节审查
3. 记录问题和建议
4. 区分必须修改和建议改进
5. 及时完成审查
| 标记 | 含义 | 行动 |
|---|
| MUST | 必须修改 | 阻塞合并 |
| SHOULD | 建议修改 | 强烈建议 |
| NICE | 可选改进 | 作者决定 |
| IDEA | 思考建议 | 仅讨论 |
| 反模式 | 说明 | 改进 |
|---|
| 橡皮图章 | 不仔细看就批准 | 认真审查每行代码 |
| 吹毛求疵 | 只关注格式问题 | 关注设计和逻辑 |
| 延迟审查 | 拖延审查时间 | 24小时内完成 |
| 对抗性评论 | 攻击性语言 | 建设性表达 |
| 过度设计 | 要求过度抽象 | 适度设计 |