名称: 代码审查卓越描述: 掌握有效的代码审查实践，提供建设性反馈，早期捕捉bug，促进知识共享，同时维护团队士气。在审查拉取请求、建立审查标准或指导开发者时使用。版本: 0.1.0

代码审查卓越

通过建设性反馈、系统化分析和协作改进，将代码审查从把关转变为知识共享。

何时使用此技能

审查拉取请求和代码变更
为团队建立代码审查标准
通过审查指导初级开发者
进行架构审查
创建审查清单和指南
改善团队协作
减少代码审查周期时间
维护代码质量标准

核心原则

1. 审查心态

代码审查的目标：

捕捉bug和边界情况
确保代码可维护性
在团队中分享知识
执行编码标准
改进设计和架构
构建团队文化

非目标：

炫耀知识
挑剔格式（使用linter）
不必要地阻碍进度
按个人偏好重写

2. 有效反馈

良好反馈的特征：

具体且可执行
教育性而非评判性
聚焦于代码而非个人
平衡（也表扬好工作）
优先级化（关键 vs 可有可无）

❌ 不良："这是错误的。"
✅ 良好："当多个用户同时访问时，这可能导致竞态条件。考虑在此处使用互斥锁。"

❌ 不良："为什么不用X模式？"
✅ 良好："是否考虑过Repository模式？这会使测试更容易。这里有个例子：[链接]"

❌ 不良："重命名这个变量。"
✅ 良好："[小问题] 考虑使用 `userCount` 代替 `uc` 以提高清晰度。如果偏好保持，不阻止。"

3. 审查范围

审查内容：

逻辑正确性和边界情况
安全漏洞
性能影响
测试覆盖和质量
错误处理
文档和注释
API设计和命名
架构适配

手动不审查内容：

代码格式（使用Prettier、Black等）
导入组织
代码检查违规
简单拼写错误

审查过程

阶段1: 上下文收集（2-3分钟）

在深入代码前，了解：

1. 阅读PR描述和链接的issue
2. 检查PR大小（>400行？要求拆分）
3. 审查CI/CD状态（测试通过？）
4. 理解业务需求
5. 注意相关架构决策

阶段2: 高层次审查（5-10分钟）

1. **架构与设计**
   - 解决方案是否适合问题？
   - 是否有更简单的方法？
   - 是否与现有模式一致？
   - 是否可扩展？

2. **文件组织**
   - 新文件是否在正确位置？
   - 代码是否逻辑分组？
   - 是否有重复文件？

3. **测试策略**
   - 是否有测试？
   - 测试是否覆盖边界情况？
   - 测试是否可读？

阶段3: 逐行审查（10-20分钟）

对于每个文件：

1. **逻辑与正确性**
   - 边界情况是否处理？
   - 是否有一一错误？
   - 空值/未定义检查？
   - 竞态条件？

2. **安全**
   - 输入验证？
   - SQL注入风险？
   - XSS漏洞？
   - 敏感数据暴露？

3. **性能**
   - N+1查询？
   - 不必要的循环？
   - 内存泄漏？
   - 阻塞操作？

4. **可维护性**
   - 变量名清晰？
   - 函数是否做一件事？
   - 复杂代码是否注释？
   - 魔术数字是否提取？

阶段4: 总结与决策（2-3分钟）

1. 总结关键问题
2. 突出你喜欢的部分
3. 明确决策：
   - ✅ 批准
   - 💬 评论（次要建议）
   - 🔄 请求更改（必须解决）
4. 如果复杂，提供配对协助

审查技巧

技巧1: 清单方法

## 安全清单
- [ ] 用户输入验证和清理
- [ ] SQL查询使用参数化
- [ ] 认证/授权检查
- [ ] 秘密不硬编码
- [ ] 错误消息不泄露信息

## 性能清单
- [ ] 无N+1查询
- [ ] 数据库查询索引
- [ ] 大列表分页
- [ ] 昂贵操作缓存
- [ ] 热点路径无阻塞I/O

## 测试清单
- [ ] 快乐路径测试
- [ ] 边界情况覆盖
- [ ] 错误情况测试
- [ ] 测试名称描述性
- [ ] 测试确定性

技巧2: 问题方法

代替陈述问题，提问鼓励思考：

❌ "如果列表为空，这将失败。"
✅ "如果 `items` 是空数组会发生什么？"

❌ "这里需要错误处理。"
✅ "如果API调用失败，应该如何行为？"

❌ "这效率低下。"
✅ "我看到这循环遍历所有用户。考虑过10万用户时的性能影响吗？"

技巧3: 建议而非命令

## 使用协作语言

❌ "你必须改为使用async/await"
✅ "建议：async/await可能使这更可读：
    ```typescript
    async function fetchUser(id: string) {
        const user = await db.query('SELECT * FROM users WHERE id = ?', id);
        return user;
    }
    ```
    你觉得如何？"

❌ "提取这到一个函数"
✅ "这个逻辑出现在3个地方。提取到共享工具函数是否有意义？"

技巧4: 区分严重性

使用标签表示优先级：

🔴 [阻塞] - 合并前必须修复
🟡 [重要] - 应该修复，如果不同意则讨论
🟢 [小问题] - 可有可无，不阻塞
💡 [建议] - 替代方法考虑
📚 [学习] - 教育性评论，无需行动
🎉 [表扬] - 好工作，保持！

例子：
"🔴 [阻塞] 这个SQL查询易受注入攻击。请使用参数化查询。"

"🟢 [小问题] 考虑将 `data` 重命名为 `userData` 以提高清晰度。"

"🎉 [表扬] 优秀的测试覆盖！这会捕捉边界情况。"

语言特定模式

Python代码审查

# 检查Python特定问题

# ❌ 可变默认参数
def add_item(item, items=[]):  # 错误！在所有调用中共享
    items.append(item)
    return items

# ✅ 使用None作为默认
def add_item(item, items=None):
    if items is None:
        items = []
    items.append(item)
    return items

# ❌ 捕获太广
try:
    result = risky_operation()
except:  # 捕获一切，甚至KeyboardInterrupt！
    pass

# ✅ 捕获特定异常
try:
    result = risky_operation()
except ValueError as e:
    logger.error(f"无效值: {e}")
    raise

# ❌ 使用可变类属性
class User:
    permissions = []  # 在所有实例中共享！

# ✅ 在__init__中初始化
class User:
    def __init__(self):
        self.permissions = []

TypeScript/JavaScript代码审查

// 检查TypeScript特定问题

// ❌ 使用any破坏类型安全
function processData(data: any) {  // 避免any
    return data.value;
}

// ✅ 使用正确类型
interface DataPayload {
    value: string;
}
function processData(data: DataPayload) {
    return data.value;
}

// ❌ 未处理异步错误
async function fetchUser(id: string) {
    const response = await fetch(`/api/users/${id}`);
    return response.json();  // 如果网络失败怎么办？
}

// ✅ 正确处理错误
async function fetchUser(id: string): Promise<User> {
    try {
        const response = await fetch(`/api/users/${id}`);
        if (!response.ok) {
            throw new Error(`HTTP ${response.status}`);
        }
        return await response.json();
    } catch (error) {
        console.error('获取用户失败:', error);
        throw error;
    }
}

// ❌ 属性突变
function UserProfile({ user }: Props) {
    user.lastViewed = new Date();  // 突变属性！
    return <div>{user.name}</div>;
}

// ✅ 不要突变属性
function UserProfile({ user, onView }: Props) {
    useEffect(() => {
        onView(user.id);  // 通知父级更新
    }, [user.id]);
    return <div>{user.name}</div>;
}

高级审查模式

模式1: 架构审查

审查重大变更时：

1. **先设计文档**
   - 对于大型功能，在代码前请求设计文档
   - 在实现前与团队审查设计
   - 就方法达成一致，避免返工

2. **分阶段审查**
   - 第一个PR：核心抽象和接口
   - 第二个PR：实现
   - 第三个PR：集成和测试
   - 更容易审查，更快迭代

3. **考虑替代方案**
   - "考虑过使用[模式/库]吗？"
   - "与更简单方法的权衡是什么？"
   - "随着需求变化，这将如何演变？"

模式2: 测试质量审查

// ❌ 不良测试：实现细节测试
test('增加计数器变量', () => {
    const component = render(<Counter />);
    const button = component.getByRole('button');
    fireEvent.click(button);
    expect(component.state.counter).toBe(1);  // 测试内部状态
});

// ✅ 良好测试：行为测试
test('点击时显示递增计数', () => {
    render(<Counter />);
    const button = screen.getByRole('button', { name: /增加/i });
    fireEvent.click(button);
    expect(screen.getByText('计数: 1')).toBeInTheDocument();
});

// 测试审查问题：
// - 测试是否描述行为而非实现？
// - 测试名称是否清晰描述性？
// - 测试是否覆盖边界情况？
// - 测试是否独立（无共享状态）？
// - 测试是否可按任何顺序运行？

模式3: 安全审查

## 安全审查清单

### 认证与授权
- [ ] 需要认证的地方是否要求？
- [ ] 每个操作前是否有授权检查？
- [ ] JWT验证是否适当（签名、过期）？
- [ ] API密钥/秘密是否安全保护？

### 输入验证
- [ ] 所有用户输入验证？
- [ ] 文件上传限制（大小、类型）？
- [ ] SQL查询参数化？
- [ ] XSS保护（转义输出）？

### 数据保护
- [ ] 密码哈希（bcrypt/argon2）？
- [ ] 敏感数据静止加密？
- [ ] 敏感数据强制执行HTTPS？
- [ ] PII处理符合法规？

### 常见漏洞
- [ ] 无eval()或类似动态执行？
- [ ] 无硬编码秘密？
- [ ] 状态更改操作的CSRF保护？
- [ ] 公共端口的速率限制？

给出困难反馈

模式: 三明治方法（修改版）

传统：表扬 + 批评 + 表扬（感觉假）

更好：上下文 + 具体问题 + 有帮助的解决方案

例子：
"我注意到支付处理逻辑内联在控制器中。这使得测试和重用更困难。

[具体问题]
calculateTotal()函数混合了税收计算、折扣逻辑和数据库查询，难以单元测试和推理。

[有帮助的解决方案]
能否提取到PaymentService类？那会使其可测试和可重用。如果需要，我可以与你配对。"

处理分歧

当作者不同意你的反馈时：

1. **寻求理解**
   "帮助我理解你的方法。什么导致你选择这个模式？"

2. **承认有效点**
   "关于X，那是个好点。我没考虑到那个。"

3. **提供数据**
   "我担心性能。我们能添加基准测试来验证方法吗？"

4. **如果需要升级**
   "让[架构师/高级开发]来权衡这个。"

5. **知道何时放手**
   如果它工作且非关键问题，批准它。完美是进步的敌人。

最佳实践

及时审查：24小时内，理想当天
限制PR大小：最大200-400行以有效审查
时间块审查：最多60分钟，休息
使用审查工具：GitHub、GitLab或专门工具
自动化可能内容：linter、formatter、安全扫描
建立关系：表情符号、表扬和共情重要
可用性：提供配对处理复杂问题
向他人学习：审查他人的审查评论

常见陷阱

完美主义：为次要风格偏好阻止PRs
范围蔓延：“趁此机会，你能不能也…”
不一致：对不同人不同标准
延迟审查：让PRs坐几天
幽灵化：请求更改后消失
橡皮图章：未经实际审查批准
自行车棚：广泛争论琐碎细节

模板

PR审查评论模板

## 总结
[简要概述审查内容]

## 优势
- [做得好之处]
- [好模式或方法]

## 必需更改
🔴 [阻塞问题1]
🔴 [阻塞问题2]

## 建议
💡 [改进1]
💡 [改进2]

## 问题
❓ [需要澄清X]
❓ [替代方法考虑]

## 裁决
✅ 解决必需更改后批准

资源

references/code-review-best-practices.md：综合审查指南
references/common-bugs-checklist.md：语言特定bug注意点
references/security-review-guide.md：安全焦点审查清单
assets/pr-review-template.md：标准审查评论模板
assets/review-checklist.md：快速参考清单
scripts/pr-analyzer.py：分析PR复杂度并建议审查者

名称: 代码审查卓越 描述: 掌握有效的代码审查实践，提供建设性反馈，早期捕捉bug，促进知识共享，同时维护团队士气。在审查拉取请求、建立审查标准或指导开发者时使用。 版本: 0.1.0

代码审查卓越

何时使用此技能

核心原则

1. 审查心态

2. 有效反馈

3. 审查范围

审查过程

阶段1: 上下文收集（2-3分钟）

阶段2: 高层次审查（5-10分钟）

阶段3: 逐行审查（10-20分钟）

阶段4: 总结与决策（2-3分钟）

审查技巧

技巧1: 清单方法

技巧2: 问题方法

技巧3: 建议而非命令

技巧4: 区分严重性

语言特定模式

Python代码审查

TypeScript/JavaScript代码审查

高级审查模式

模式1: 架构审查

模式2: 测试质量审查

模式3: 安全审查

给出困难反馈

模式: 三明治方法（修改版）

处理分歧

最佳实践

常见陷阱

模板

PR审查评论模板

资源

名称: 代码审查卓越描述: 掌握有效的代码审查实践，提供建设性反馈，早期捕捉bug，促进知识共享，同时维护团队士气。在审查拉取请求、建立审查标准或指导开发者时使用。版本: 0.1.0