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 レート制限、大規模なデータセット、ページネーションの境界
• 安全性チェック：すべての変更を伴うコマンド（create、edit、apply、delete、update）は、LoadConfig() の後、クライアント操作の前に安全性チェックを含める必要があります。パターン：

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

適切な操作タイプであることを確認してください。ドライランのパスでのみ省略してください。

• ライブラリコードでの stdout 出力禁止：pkg/ は出力を返し、表示してはいけません。出力を扱うのは cmd/ のみです。
• ハードコードされた秘密情報や顧客データの禁止：コードやテストに実在する名前、環境 ID、トークン、メールアドレスを含めないでください。メールアドレスには @example.invalid を使用してください（RFC 2606）。

2. コード品質

コードはクリーンで、慣用的で、保守しやすいですか？

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

目視では見落としがちな問題を見つけるため、リンターを実行します。

make lint-strict

3. ユーザー体験

ユーザーの視点から見て、使い心地は適切ですか？

• コマンド名：dtctl <verb> <resource> のパターンに従っている。独自のクエリフラグは使わず、DQL パススルーを使用する。
• 出力形式：テーブル出力は読みやすく、列は適切で、ずれがない。JSON/YAML 出力は整っている。
• エラーメッセージ：明確で、実行可能な内容であり、次の手順を提案する。エージェントモード（-A）では、機械可読コードを含む構造化 JSON でエラーを返す。
• 対話動作：名前解決と候補の絞り込みプロンプトが機能する。--plain は対話動作を無効にする。
• ヘルプテキスト：コマンドに Short 説明、Long 説明、Example フィールドがある。親の動詞コマンドにも例がある。
• エイリアス：リソースに適切なエイリアスがある（例：ワークフローには wf、ダッシュボードには 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)

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