engineering-small-prs

小規模な PR の作成

小規模な PR は、徹底的にレビューしやすく、安全にマージでき、ロールバックも容易です。また、方針が変わった場合に作業が無駄になる可能性も低くなります。「小規模」とは、単に行数が少ないことではなく、概念的に焦点が絞られ、自己完結していることを意味します。

Google Engineering Practices Documentation、特に「Small CLs」を参考にしています。出典：https://google.github.io/eng-practices/review/developer/small-cls.html。ライセンス：CC-BY 3.0。

適切な小規模 PR の定義

適切な PR は、次の条件を満たします。

• 自己完結した変更を 1 つ行います。
• その変更を検証するために必要なテストを含みます。
• マージ後もシステムが正常に動作します。
• PR、説明、既存のコード、または先にレビューされた PR で、レビュー担当者が十分な背景情報を得られます。
• 動作を理解できないほど小さくありません。通常、新しい API には、同じ PR 内で少なくとも 1 つの使用例が必要です。
• 整理がごく局所的な場合を除き、大規模なリファクタリングと動作変更を分けます。

レビュー担当者は、適切にレビューするには大きすぎるという理由だけで、妥当に PR を却下できます。

分割ワークフロー

1. 最終的な成果を 1 文で表します。
2. 独立した動作変更、リファクタリング、スキーマ／設定変更、テスト、ドキュメントを列挙します。
3. 先に取り込む必要がある依存関係を特定します。
4. 以下のセクションから分割方法を選びます。
5. どの PR をマージしてもビルドが成功するように、マージ順序を組み立てます。
6. 各 PR が大きな作業の中でどのような位置づけなのかをレビュー担当者が理解できるよう、背景情報を記載します。
7. リスクが高い、または避けられない大規模な変更の場合は、PR を送る前にレビュー担当者の了承を得ます。

分割方法

• 準備として先にテストを追加する：リスクのあるリファクタリングの前に、既存の動作を対象とする不足しているテストを追加します。
• 先にリファクタリングする：動作を変えずに、コードの移動、名前変更、抽出、簡素化を行います。
• 垂直スライス：ユーザーまたは API から見える動作を 1 つ、端から端まで小さく提供します。
• 水平スライス：共有インターフェース、プロトコル定義、アダプターを追加する場合は、同じ PR 内で実際に最初の使用例と組み合わせるか、明確に関連づけられた依存 PR がすでにレビュー中の場合に限ります。
• ファイルまたは担当者ごとのグループ：変更がそれ以外の点で自己完結している場合は、レビュー担当者の専門分野に応じてファイルを分けます。
• 設定は別にする：ロールバックのタイミングが異なる場合は、ロールアウト設定、実験、デプロイ構成をコードから分けます。
• 生成された変更や機械的な変更は別にする：信頼できる自動書き換えを、手書きのロジックから分離します。
• 必要に応じて削除を別にする：使われていないコードの削除は、簡潔でレビューしやすい PR にできます。

移行の安全性

スキーマ、データ、API、またはインフラストラクチャの移行では、拡張／縮小の順序を推奨します。

1. 拡張：古い動作を削除せず、後方互換性のあるスキーマ／API／設定のサポートを追加します。
2. 二重経路：必要な場合に限り、乖離を監視できるようにして、両方の経路で書き込みまたは読み取りを行います。
3. バックフィル：進捗追跡、再試行動作、検証を備えて既存データを移行します。
4. 切り替え：検証によって両方の経路が安全だと確認できた後に、トラフィックまたは呼び出し元を切り替えます。
5. 縮小：後片付けの PR で、古いスキーマ、コード経路、フラグ、互換性シムを削除します。

各移行 PR では、デプロイ順序、ロールバック動作、検証シグナル、および次の PR がマージされるまで行ってはいけないことを明記します。

出力テンプレート

## Goal
[Final outcome]

## Proposed PR Sequence
| # | PR | Purpose | Depends On | Tests | Reviewer Notes |
|---|----|---------|------------|-------|----------------|
| 1 | Add characterization tests for ... | Protect existing behavior before refactor | None | ... | ... |
| 2 | Extract ... without behavior change | Prepare smaller implementation | 1 | ... | ... |
| 3 | Implement ... vertical slice | User-visible behavior | 2 | ... | ... |

## Migration Safety
- Expand/contract phase:
- Backfill/validation:
- Observability:
- Rollback:

## Build Safety
- Each PR keeps:
- Rollback notes:

## Large-Change Risk
- What could not be split:
- Reviewer consent needed:

判断の目安

• 必要だと思うより小さな PR を優先します。焦点が絞られた変更をレビュー担当者が問題視することは、ほとんどありません。
• 小さな論理的変更が多数のファイルにまたがる場合は、行数が示すより大きな変更として扱います。
• PR を理解するために多くの将来の背景情報が必要なら、手順に先行する準備用 PR が欠けているか、説明を改善する必要がある可能性があります。
• PR を分割できない場合は、テストを強化し、説明を明確にし、リスクを明示して、事前にレビュー担当者の了承を得るなど、レビューしやすさを特に高めます。

よくある間違い

• 広範なフォーマット変更と機能ロジックを混在させる。
• 実際の使用例がない抽象化を取り込む。
• 中間の PR でビルドが壊れるスタック構成を作る。
• リスクのある動作変更を「リファクタリング」PR に隠す。
• 安全なスタック型ワークフローを利用できるのに、次の PR に着手するのを 1 つ目の PR のマージ後まで待つ。