pr-review

PR の品質レビュー

プルリクエストまたはフィーチャーブランチを包括的に品質レビューします。このスキルでは、本番環境への対応状況、コード品質、UX、ドキュメント、テスト、安全性を確認します。

レビューは、7 つの観点からなるチェックリストに沿って進めます。各観点について、実際のコードを調査し、具体的な所見を報告してください。「問題ありません」のようなコメントだけではなく、ファイルパスと行番号を示してください。

はじめに

まず、変更の範囲を把握します。

# What branch are we on, what's the base?
git branch --show-current
git log main..HEAD --oneline

# What files changed?
git diff main...HEAD --stat

# Full diff for review
git diff main...HEAD

リモート PR をレビューする場合は、まず取得します。

gh pr view <number> --json title,body,files
gh pr diff <number>

コードを詳しく調べる前に、PR の説明とすべてのコミットを読んで、意図を把握してください。

レビューの観点

以下の各観点について確認し、ステータスを報告してください。

• 合格 -- 基準を満たしており、問題はありません
• 要対応 -- 具体的な問題があります（内容を列挙してください）
• 該当なし -- 今回の変更には該当しません

1. 本番環境への対応状況

このコードは正しく動作し、障害に適切に対処できますか？

• エラー処理：すべてのエラーを確認していますか？コンテキストを付けてラップしていますか（fmt.Errorf("context: %w", err)）？ユーザーに対して、対応可能なメッセージを表示していますか？
• エッジケース：空の入力、nil 値、設定の欠落、ネットワーク障害、API のレート制限、大規模なデータセット、ページネーションの境界
• 安全性チェック：すべての変更系コマンド（作成、編集、適用、削除、更新）では、LoadConfig() の後、クライアント操作の前に安全性チェックを行う必要があります。パターン：

  checker, err := NewSafetyChecker(cfg)
  if err := checker.CheckError(safety.OperationXXX, safety.OwnershipUnknown); err != nil { return err }
  

  正しい操作タイプであることを確認してください。ドライランの経路に限り、省略できます。
• ライブラリコードで標準出力を使わない：pkg/ はデータを返し、出力しないでください。出力を処理するのは cmd/ のみです。
• ハードコードされた秘密情報や顧客データを含めない：コードやテストに、実在する氏名、環境 ID、トークン、メールアドレスを含めないでください。メールアドレスには @example.invalid（RFC 2606）を使用してください。

2. コード品質

コードは簡潔で、慣用的で、保守しやすいですか？

• Go の慣用表現：Effective Go と Go Code Review Comments に従っている
• 命名：説明的な名前で、Go の規則に従っている（非公開は camelCase、公開は PascalCase）。インターフェースには -er 接尾辞を使用する
• ファイルサイズ：ファイルは 500 行未満にする。大きなファイルは分割する
• インポート：標準ライブラリ、サードパーティ、内部（github.com/dynatrace-oss/dtctl）の順
• 重複：ヘルパーに切り出すべき、コピー＆ペーストされたコードがないか確認する
• コメント：公開関数や型にはドキュメントを付ける。コメントでは「何を」ではなく「なぜ」を説明する
• 一貫したパターン：新しいコードはコードベース内の既存パターンに従う。参考実装として pkg/resources/ の類似リソースを確認する

目視では見つけにくい問題を検出するため、リンターを実行します。

make lint-strict

3. ユーザーエクスペリエンス

ユーザーの視点から見て、自然な使い心地ですか？

• コマンドの命名：dtctl <verb> <resource> のパターンに従う。独自のクエリフラグは使わず、DQL のパススルーを使用する
• 出力形式：テーブル出力が読みやすく、列の内容が適切で、配置がずれていない。JSON/YAML 出力が整っている
• エラーメッセージ：明確で、対応方法が分かり、次の手順を示している。エージェントモード（-A）では、エラーが機械可読コードを含む構造化 JSON になっている
• 対話的な動作：名前の解決や選択肢の提示が適切に機能する。--plain で対話的な動作が無効になる
• ヘルプテキスト：コマンドに Short、Long、Example フィールドの説明がある。親動詞コマンドにも例がある
• エイリアス：リソースに適切なエイリアスがある（例：workflow の wf、dashboard の dash）
• エージェントモード：コマンドがコンテキストに応じた提案を含む --agent エンベロープをサポートする。-A -o json でテストする
• 色の制御：NO_COLOR、FORCE_COLOR、--plain、TTY の検出に対応している

実際のコマンドを実行して、使い心地を確認してください。

# Does the help text look good?
dtctl <command> --help

# Does table output look right?
dtctl <command> --plain

# Does agent mode work?
dtctl <command> -A

4. テストカバレッジ

変更は十分にテストされていますか？

• ユニットテスト：新しい関数にはテストを追加します。テーブル駆動テストを推奨します。
• カバレッジ目標：全体で最低 70％、新しいパッケージで 80％、重要なパッケージ（pkg/client、pkg/config）で 90％です。
• エッジケースのテスト：正常系だけでなく、エラー条件、空の入力、境界値もテストします。
• モックサーバーのガード：ページネーションを扱うモックサーバーでは、無効なパラメーターの組み合わせ（例：page-size と page-key）を拒否する必要があります。Settings API のモックでも、nextPageKey と組み合わせた schemaIds／scopes を拒否する必要があります。
• ゴールデンテスト：出力形式が変更された場合や新しいリソースが追加された場合は、ゴールデンファイルを更新する必要があります。以下を確認してください：

  go test ./pkg/output/ -run TestGolden
  

  ゴールデンテストでは、pkg/resources/* の実際の本番用構造体を使用してください。テスト専用の複製は使用しないでください。
• E2E テスト：新しいリソースや複雑なワークフローについては、test/e2e/ に統合シナリオを追加します。

# Run full suite
go test ./...

# Check coverage
make test-coverage

5. ドキュメント

ユーザーやコントリビューター向けに、変更が適切に文書化されていますか？

常に必須：

• Conventional Commits 形式のタイトル：PR のタイトル（および squash merge のコミット）は、有効な Conventional Commits 形式（feat: ...、fix: ...、破壊的変更の場合は feat!: ...）である必要があります。release-please はこの情報からバージョンの更新内容と GitHub Release のリリースノートを生成します。編集が必要な CHANGELOG.md はありません。

新機能で必須：

• README.md：機能がユーザー向けで重要な場合（新しいリソースタイプ、新しいコマンドカテゴリなど）は更新します。
• docs/QUICK_START.md：主要な新機能の使用例を追加します。
• docs/dev/IMPLEMENTATION_STATUS.md：機能一覧の行を更新します。
• docs/dev/API_DESIGN.md：新しい規約を導入する場合は、設計パターンを文書化します。
• docs/TOKEN_SCOPES.md：機能で追加の API 権限が必要な場合は、新しいスコープを文書化します。

新しいリソースで必須：

• docs/resources/ にリソース固有のドキュメントページを追加し、ジェネレーターがそのテーブルを管理できるよう、scripts/gen-docs/gen_all.py に PLAN エントリを追加します。
• コマンドリファレンス：手作業で編集せず、再生成します。make docs-generate により docs/COMMANDS.md が書き換えられます。

新しい AI エージェントのサポートで必須：

• README.md、docs/QUICK_START.md、docs/dev/API_DESIGN.md、docs/dev/IMPLEMENTATION_STATUS.md（4 つすべて）

6. ドキュメントのチェック

ドキュメントサイトはありません。docs/site/ は廃止され、以前公開された URL を docs/ にリダイレクトするスタブだけが残っています。docs/site/_docs/ にページを追加する PR は誤りです。

確認事項：

• 生成ブロックを手作業で変更していない：`` 内を編集していないこと。カタログに変更があった場合は、make docs-generate を実行し、その出力をコミットしていること。
• 新しいリソースのマッピング：新しいカタログリソースが scripts/gen-docs/INDEX.md のいずれかのページに記載され、未マッピング一覧には含まれていないこと。
• 文章を検証済み：make docs-check が成功すること。文書に記載されたすべての dtctl ... が解決され、すべてのフラグが存在する必要があります。CI でも実行されますが、レビュー担当者は新しい例が実際に実行されたものであり、作り上げられたものではないことを確認してください。
• リンク：相対リンクが解決され、アンカーが存在すること（make docs-check で確認できます）。

7. PR の説明

PR 自体が適切に説明されていますか？

• タイトル：明確で、Conventional Commits 形式（feat: ...、fix: ...）に従っていること
• 概要：何が変わったか、なぜ変わったかを説明していること（変更したファイルの説明だけでは不十分です）
• 関連する Issue：Closes #NNN または Fixes #NNN で Issue を参照していること
• 破壊的変更：該当する場合は明示されていること
• テストセクション：変更をどのようにテストしたかを説明していること
• UX の例：ユーザー向けの変更では、変更前と変更後の CLI 出力を示していること

レビュー結果

レビュー完了後、次の形式で概要を提示してください：

## PR Review: <title>

| Dimension | Status | Notes |
|-----------|--------|-------|
| Production readiness | Pass/Needs work | ... |
| Code quality | Pass/Needs work | ... |
| User experience | Pass/Needs work | ... |
| Test coverage | Pass/Needs work | ... |
| Documentation | Pass/Needs work | ... |
| GitHub Pages | Pass/Needs work/N/A | ... |
| PR description | Pass/Needs work | ... |

### Issues Found
1. **[Dimension]** file:line -- description of issue
2. ...

### Suggestions (non-blocking)
1. ...

### Verdict
Ready to merge / Needs revisions (list blockers)

率直かつ具体的に記述してください。正確なファイルパスと行番号を示してください。ブロッキングの問題（修正必須）と提案（あると望ましい）を区別してください。称賛でレビューを水増しせず、対応が必要な点に集中してください。