代码审查Skill code-review

---

name: 代码审查 description: 执行代码审查，遵循Sentry工程实践。用于审查拉取请求、检查代码更改或提供代码质量反馈。涵盖安全、性能、测试和设计审查。

Sentry 代码审查

遵循这些指南来审查Sentry项目的代码。

审查清单

识别问题

在代码更改中寻找这些问题：

• 运行时错误：潜在异常、空指针问题、越界访问
• 性能：无限制的O(n²)操作、N+1查询、不必要的分配
• 副作用：无意中影响其他组件的行为更改
• 向后兼容性：破坏API变更而没有迁移路径
• ORM查询：复杂Django ORM导致意外查询性能问题
• 安全漏洞：注入、XSS、访问控制漏洞、密钥暴露

设计评估

• 组件交互是否逻辑合理？
• 更改是否与现有项目架构一致？
• 是否存在与当前要求或目标的冲突？

测试覆盖

每个PR应该有适当的测试覆盖：

• 业务逻辑的功能测试
• 组件交互的集成测试
• 关键用户路径的端到端测试

验证测试覆盖实际要求和边缘情况。避免测试代码中的过度分支或循环。

长期影响

当更改涉及以下内容时，标记给高级工程师审查：

• 数据库模式修改
• API合约更改
• 新框架或库采用
• 性能关键代码路径
• 安全敏感功能

反馈指南

语气

• 礼貌且同理心
• 提供可操作的建议，而非模糊批评
• 不确定时以问题形式表达：“您是否考虑过…？”

批准

• 当仅剩次要问题时批准
• 不要因风格偏好而阻塞PR
• 记住：目标是降低风险，而非完美代码

常见模式标记

Python/Django

# 差：N+1查询
for user in users:
    print(user.profile.name)  # 每个用户单独查询

# 好：预取相关
users = User.objects.prefetch_related('profile')

TypeScript/React

// 差：useEffect中缺少依赖
useEffect(() => {
  fetchData(userId);
}, []);  // userId不在依赖中

// 好：包含所有依赖
useEffect(() => {
  fetchData(userId);
}, [userId]);

安全

# 差：SQL注入风险
cursor.execute(f"SELECT * FROM users WHERE id = {user_id}")

# 好：参数化查询
cursor.execute("SELECT * FROM users WHERE id = %s", [user_id])

参考文献

• Sentry 代码审查指南