code-review

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 代码审查指南