code-review

Sentry コードレビュー

Sentry のプロジェクトのコードをレビューする際は、以下のガイドラインに従ってください。

レビューチェックリスト

問題の特定

コード変更で以下の問題がないか確認してください:

• ランタイムエラー: 例外の発生可能性、null ポインター問題、範囲外アクセス
• パフォーマンス: 制限のない O(n²) 操作、N+1 クエリ、不要なメモリ割り当て
• 副作用: 他のコンポーネントに影響する意図しない動作の変更
• 後方互換性: 移行パスのない破壊的な API 変更
• ORM クエリ: 予期しないクエリパフォーマンスを引き起こす複雑な Django ORM
• セキュリティ脆弱性: インジェクション、XSS、アクセス制御の不備、シークレットの漏洩

設計評価

• コンポーネント間の相互作用は論理的に妥当か?
• 変更は既存のプロジェクトアーキテクチャと整合しているか?
• 現在の要件や目標と矛盾はないか?

テストカバレッジ

すべての PR には適切なテストカバレッジが必要です:

• ビジネスロジックに対する機能テスト
• コンポーネント間の相互作用に対する統合テスト
• 重要なユーザーパスに対するエンドツーエンドテスト

テストが実際の要件とエッジケースをカバーしているか確認してください。テストコードでは過度な分岐やループを避けてください。

長期的な影響

以下を含む変更は、シニアエンジニアによるレビューを依頼してください:

• データベーススキーマの変更
• API コントラクトの変更
• 新しいフレームワークやライブラリの採用
• パフォーマンスが重要なコードパス
• セキュリティに関わる機能

フィードバックのガイドライン

トーン

• 丁寧で共感的であること
• 曖昧な批判ではなく、実行可能な提案を行うこと
• 確信が持てない場合は質問の形で伝えること: 「...を検討されましたか?」

承認

• 残っている問題が軽微な場合は承認すること
• スタイルの好みを理由に PR をブロックしないこと
• 目標はリスクの低減であり、完璧なコードではないことを忘れないこと

指摘すべき一般的なパターン

Python/Django

# Bad: N+1 query
for user in users:
    print(user.profile.name)  # Separate query per user

# Good: Prefetch related
users = User.objects.prefetch_related('profile')

TypeScript/React

// Bad: Missing dependency in useEffect
useEffect(() => {
  fetchData(userId);
}, []);  // userId not in deps

// Good: Include all dependencies
useEffect(() => {
  fetchData(userId);
}, [userId]);

セキュリティ

# Bad: SQL injection risk
cursor.execute(f"SELECT * FROM users WHERE id = {user_id}")

# Good: Parameterized query
cursor.execute("SELECT * FROM users WHERE id = %s", [user_id])

参考資料

• Sentry コードレビューガイドライン