tomevault-io Code Review
コードレビューを支援するスキル。コードベースの健康状態を時間をかけて改善することを目的とし、レビューの観点、進め方、コメントの書き方を提供する。
主な機能:
- レビュー観点の確認（設計、機能性、複雑性、テスト等）
- レビューの進め方（投稿前準備、レビュー実施、フィードバック対応）
- コメントの書き方（礼儀正しさ、Nit、説明）
- 効率的なレビューのための定量的ガイドライン
- 意見対立の解消方法
概要
コードレビューが達成すべき目標：
- コードが要件に対して正しく効果的な解決策であることを確認する
- コードが保守可能であることを保証する
- コードベースに関する知識を共有する
- 定期的なフィードバックを通じてチームのスキルを向上させる
- 開発者の時間に過度の負担をかけない
最上位原則（CRITICAL）:
CLが完璧でなくても、その変更がシステムのコードの全体的な健康状態を改善すると確実にわかれば、レビュアーはCLを積極的に承認すべきである。
「完璧な」コードは存在しない。存在するのは「より良い」コードだけ。
使用タイミング
以下の場合にこのスキルを読み込む：
- コードレビューを行う場合
- レビューコメントを書く場合
- CLの品質を評価する場合
- レビューの進め方を確認する場合
レビュー投稿時（When Posting a Code Review）
適切なレビュアーを選ぶ
- コードが正しいか、アーキテクチャが適切か、規約に準拠しているかを確認できる人を選ぶ
- レビュアーがボトルネックになっている場合は、より適切なレビュアーを探すかタイムラインを調整する
必ずプライマリレビュアーを置く
- プライマリレビュアーはコードレビュー全体に責任を持つ
- コードを書いた人と同等の責任を持つ
- 全員が誰が最終責任を持つかを知れるよう、明示的に指定する
- 緊急時を除き、プライマリレビュアーの承認なしにコードをシップしない
各レビュアーの責任を明確に
- 複数人がレビューする場合、各人の担当領域を明確にする
- 重複を避け、議論を管理可能に保つ
コミュニケーションが鍵
- 変更のコンテキストをレビュアーに提供する
- 重要なレビューの前には話し合うか、レビューの説明文書に記載する
- 変更内容、変更理由、チケット/仕様へのリンクを含める
- タイムラインを設定する
コード準備（Preparing Your Code for Review）
小さいほど良い
- コードレビューを小さく保つことで、より速く正確に反復できる
- 小さなコード変更はテストや検証も容易
- ロジックの変更とスタイルの変更は別のレビューに分ける
レビューしやすいコードにする
- 高レベルのコメントとインラインコメントの両方を含める
- 高レベル: コンポーネントがどう連携するか、例外ケースの処理
- インライン: なぜそのコードの形になっているか
- 混乱する場合は、開始点と無視できる部分を明示する
自分でdiffを読んでから投稿する
- 初めて見る人の視点で確認する
- 混乱を招く可能性のある決定を探す
- 問題を早期に発見すれば、自分とレビュアーの時間を節約できる
レビュー中（During a Code Review）
レビュー中の大きな変更を避ける
- レビュー中の大きな変更は、レビュープロセスをリセットする
- 大きな変更が必要な場合は、既存のレビューをシップしてから追加変更を行う
- 「ドラフト」レビューで事前フィードバックを求める場合は明示的に伝える
すべてのフィードバックに対応する
- すべてのフィードバックを理解し、各実行可能な指摘に応答する
- フィードバックを実装しない場合でも、応答して理由を説明する
- ほとんどのコードレビューフィードバックは「命令」ではなく「提案」と考える
レビュー時（When Reviewing Code）
レビューしたコードに責任を持つ
コードを書いた人と同等の責任を持つ。
- コードが正しいか、アーキテクチャが適切か、安全か、保守可能かを保証する
- 基準を満たしていると確信できない場合は「ship it」を出さない
- 不確かな場合はチームメイトに助けを求める
時間が重要
- 可能な限り速くレビューを返す
- 理想的には24時間以内に返す（プロジェクトの勢いを維持するため）
- 大きなレビューの場合、一度にすべてを完了する必要はない
- すべてをレビューするまでコードをシップしない
正確性をレビューする
- バグがないか確認
- 意図した問題を解決しているか
- エッジケースを適切に処理しているか
- データのシリアライズ/デシリアライズの場合、ロールバック/ロールフォワード安全か
コアロジックとアーキテクチャをレビューする
- 各部分を理解し、それらがどう連携するか確認
- 各コンポーネントのロジックが効率的か
- アーキテクチャが柔軟だがオーバーエンジニアリングされていないか
- 迷ったら、可読性と保守性を優先する
適切なテストカバレッジを確認する
- すべてのコードはテストされるべき
- ユニットテスト、結合テスト、回帰テスト等
- ユニットテストが適切に分離され、不必要な依存関係がないか
セキュリティを確認する
- セキュリティホールがないかチェック
- 不確かな場合はセキュリティチームに相談
効率的なコードレビューの10のヒント（SmartBear研究）
1. 一度に400行以下のコードをレビューする
SmartBearのCisco Systems調査によると、**200〜400行（LOC）**が一度のレビューに最適。
- 脳が一度に処理できる情報量には限界がある
- 400行を超えると欠陥発見能力が低下する
- 200-400行のレビューを60-90分かけて行うと、70-90%の欠陥発見率が期待できる
2. 検査速度は1時間あたり500行未満
500行/時間を超える速度では、欠陥密度が大幅に増加する。
- 「他の人が見つけてくれるだろう」と考えてレビューを急ぐべきではない
- 適切な量のコードレビューを、一定の時間をかけて実施することが最も効果的
3. 1回のレビュー時間は60分以内
約60分を過ぎるとパフォーマンスが低下し始める。
- 集中を要する作業では、定期的な休憩が品質向上につながる
- より頻繁なレビューを実施すれば、長時間レビューの必要性が減る
4. 目標を設定し、指標を収集する
SMART基準を用いて具体的な目標を設定：
- 外部指標例：「サポート問い合わせ件数を15%削減」「開発段階の欠陥率を半減」
- 内部プロセス指標：
- レビュー実施速度
- 欠陥率（レビュー1時間あたりのバグ数）
- 欠陥密度（コード1行あたりのバグ数）
5. レビュー前に著者がソースコードに注釈を付ける
- 注釈はレビュアーのガイドとして機能
- どのファイルを確認すべきか、各変更の根拠を明確化
- 著者自身がレビュー開始前にエラーを発見できる可能性が高まる
6. チェックリストを活用する
- チームメンバー全員が繰り返し同じ種類のミスを犯している可能性が高い
- 「見落とし」は最も発見が困難な欠陥（存在しないものをレビューするのは難しい）
- チェックリストは頻繁に発生するエラーを排除する最も効果的な方法
7. 発見された不具合を修正するプロセスを確立する
- レビュアーがバグを記録し、著者と議論し、変更を承認できる仕組みを作る
- コードレビューで発見されたバグは、標準的な不具合追跡システムに登録されないケースが多い
8. ポジティブなコードレビュー文化を醸成する
重要: コードレビューの結果を業績評価に使用すべきではない。
- 不具合は否定的な要素ではなく、コード品質を向上させる機会
- 個人の評価指標が報酬の基準となる場合、開発者は敵対的な態度を取るようになる
- 若手がベテランから学べる機会でもある
9. ピアレビューの無意識的な効果（エゴ効果）を活用する
他者が自分のコードをレビューするという意識は、自然と品質向上への動機付けとなる。
- コードの20-33%を「スポットチェック」するだけで、欠陥密度を大幅に低減できる
- コードがレビュー対象として指摘される確率が3分の1であれば、開発者は自らの作業を再確認する十分な動機付けとなる
10. 軽量なコードレビューを実施する
SmartBearの調査によると：
- 軽量コードレビューは正式なレビュー手法に比べて作業時間が20%未満でありながら、同等数のバグを発見できる
- 正式な検査では200行のコードあたり平均9時間の作業時間を要する
レビュー観点チェックリスト
1. 設計（Design）
- CLのコードの各部分は相互にきちんと連携するか
- この変更はコードベースに属するものか
- アーキテクチャが柔軟だがオーバーエンジニアリングされていないか
2. 機能性（Functionality）
- 開発者の意図通りに動作するか
- エッジケースを適切に処理しているか
- 並行処理がある場合、安全か
3. 複雑性（Complexity）
- コードが必要以上に複雑でないか
- 現在必要なものを実装しているか（将来の予測ではない）
4. テスト（Tests）
- 適切なテストカバレッジがあるか
- テストが正確で有用か
5. 命名・コメント（Naming & Comments）
- 明確な名前を使っているか
- コメントは「何」ではなく「なぜ」を説明しているか
6. セキュリティ（Security）
- セキュリティホールがないか
- ロールバック/ロールフォワード安全か
7. 保守性（Maintainability）
- このコードが3AMに壊れたとき、診断して修正できるか
コメントの書き方
礼儀正しさ（Courtesy）
必ず「コード」についてコメントし、「開発者本人」についてコメントしない。
悪い例:
並行処理にしても明らかにメリットがないのに、どうしてあなたはスレッドを使ったのですか？
良い例:
見たところ、ここで使った並行処理モデルは実際にはパフォーマンス上のメリットがないまま、ただシステムを複雑にしています。シングルスレッドのコードにするのが最善です。
Nit（あら探し）
Nit: [指摘内容]
必須ではない改善を提案する場合に使用。
「なぜ」を説明する
- コメントの意図について詳しく説明する
- 提案がコードの健康状態をどう良くするか解説する
意見の対立の解消
- 技術的な事実とデータが個人的な意見と好みをくつがえす
- スタイルに関しては、スタイルガイドが絶対的な権威
- 長い議論になる場合は、直接会話する
- オフラインでの議論は、コードレビューに要約を残す
作成者とレビュアーが合意に達することができないからといって、CLをそのまま放置しないでください。
LGTM基準
| 判定 | 基準 |
|---|---|
| LGTM | コードの健康状態が維持/改善される |
| LGTM with Comments | コメント付きで承認（後で対応 or Nit） |
| 要改善 | コードの健康状態を悪化させる |
チェックリスト
投稿前
- コンテキストと要件をレビュアーと共有したか
- プライマリレビュアーを特定したか
- 自分のコードを注意深く読んだか
- 小さなチャンクに分割できるか
- スタイルチェックをしたか
レビュー時
- コードを完全に理解したか
- 正確性を確認したか
- コアロジックとアーキテクチャを確認したか
- テストカバレッジを確認したか
- デプロイ安全性を確認したか
- セキュリティホールをチェックしたか
- 「3AMに壊れたら修正できるか」に答えられるか
リファレンス
- references/comment-templates.md - コメントテンプレート集
- references/review-checklist.md - 詳細チェックリスト
使用例
例1: 大きなCLを小さく分割する
# 悪い: 1つの巨大なCL
- ロジック変更（+500行）
- スタイル修正（+200行）
- リファクタリング（+300行）
# 良い: 分割されたCL
1. CL#1: スタイル修正のみ
2. CL#2: リファクタリング
3. CL#3: ロジック変更
例2: Nit付きのLGTM
LGTM! 以下はNit（あら探し）なので、対応は任意です：
Nit: 変数名 `data` は具体的でないかもしれませんが、
このコンテキストでは許容範囲です。
素晴らしい実装ありがとうございます！
例3: オーバーエンジニアリングの指摘
このキャッシュ機構は、現在の要件には過剰に思えます。
現在のユースケース：
- 1日あたりのリクエスト数: ~100
提案するキャッシュの処理能力：
- 1秒あたりのリクエスト数: ~10000
現在の要件に対しては、シンプルなメモリキャッシュで十分ではないでしょうか？
必要になった時に拡張することもできます。
ベストプラクティス
定量的ガイドライン
| 指標 | 推奨値 | 根拠 |
|---|---|---|
| 一度のレビュー行数 | 200-400行 | 400行超過で欠陥発見能力が低下 |
| 検査速度 | <500行/時間 | 500行/時間超過で欠陥密度増加 |
| 1回のレビュー時間 | <60分 | 60分超過でパフォーマンス低下 |
| 応答時間 | <24時間 | プロジェクト勢いの維持 |
| 欠陥発見率 | 70-90% | 適切なレビューで達成可能 |
| スポットチェック率 | 20-33% | 3分の1で十分な動機付け |
レビュー効率化のポイント
- 小さく保つ: ロジック変更とスタイル変更は分ける
- コンテキストを提供: 変更理由と関連チケットを明記
- チェックリストを使う: 頻繁なミスを排除
- 軽量プロセス: 正式な検査より20%未満の時間で同等の効果
- ポジティブな文化: 業績評価に使用しない
レビュアーとして心がけること
- コードを書いた人と同等の責任を持つ
- 「コード」についてコメントし、「人」についてコメントしない
- 「なぜ」を説明する
- 良い実践を褒める
- 完璧さではなく継続的な改善を追求する
トラブルシューティング
よくある問題
| 問題 | 解決策 |
|---|---|
| レビューが遅い | 小さなCLに分割する。24時間以内の応答を目指す |
| 意見が対立している | 直接会話する。技術的事実とデータを重視 |
| レビューが表面的 | 400行以下に制限。チェックリストを使用 |
| 開発者が不満を持っている | ポジティブな文化を醸成。業績評価に使用しない |
| 「あとで直す」が直らない | CLの承認前に今直す。TODOコメントとバグ登録 |
レビューが停滞している場合
- 原因を特定: レビュアーが忙しい、CLが大きすぎる、議論が長引いている
- 対策:
- レビュアーを追加する
- CLを分割する
- 対面で議論する
- エスカレーションする
レビュアーが見つからない場合
- チーム内でローテーションを設定する
- プライマリレビュアーを明確に指定する
- タイムラインを事前に合意する
デバッグ情報
記録されるイベント
このスキルの実行時に記録されるイベント：
| イベント種別 | 説明 | 記録タイミング |
|---|---|---|
| session_start | セッション開始 | pi起動時 |
| task_start | タスク開始 | ユーザー依頼受付時 |
| operation_start | 操作開始 | スキル実行開始時 |
| operation_end | 操作終了 | スキル実行完了時 |
| task_end | タスク終了 | タスク完了時 |
ログ確認方法
# 今日のログを確認
cat .pi/logs/events-$(date +%Y-%m-%d).jsonl | jq .
# 特定の操作を検索
cat .pi/logs/events-*.jsonl | jq 'select(.eventType == "operation_start")'
# エラーを検索
cat .pi/logs/events-*.jsonl | jq 'select(.data.status == "failure")'
トラブルシューティング
| 症状 | 考えられる原因 | 確認方法 | 解決策 |
|---|---|---|---|
| 実行が停止する | タイムアウト | ログのdurationMsを確認 | タイムアウト設定を増やす |
| 結果が期待と異なる | 入力パラメータの問題 | paramsを確認 | 入力を修正して再実行 |
| エラーが発生する | リソース不足 | エラーメッセージを確認 | 設定を調整 |
関連ファイル
- 実装:
.pi/extensions/code-review.ts - ログ:
.pi/logs/events-YYYY-MM-DD.jsonl
Converted and distributed by TomeVault — claim your Tome and manage your conversions.