Sentry コードレビュー

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

レビューチェックリスト

問題の特定

コード変更の中から以下の問題を探してください:

• ランタイムエラー: 潜在的な例外、null ポインタの問題、境界外アクセス
• パフォーマンス: 無制限の 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 Code Review Guidelines