巨大 PR の引力
リファクタリングの誘惑は「1 つの PR で全部直したい」という気持ちから始まる。コードを触り始めると関連箇所が芋づる式に出てきて、気付いたら 50 ファイル / 800 行差分の PR ができている。
これは本人にしかレビューできない。マージするのも怖い。Pull Request の体をなさない。
ある土曜日に丸 1 日かけて行ったリファクタリングを、5 個の PR に分割した記録 を書いておく。
元のコード
題材は AetherEchoes のサーバーサイド。Posts::SearchService という Service クラスが 200 行に肥大化していて、以下の責務を全部抱えていた:
- query string の parse
- カテゴリフィルタ
- タグフィルタ
- 全文検索（MySQL FULLTEXT）
- 関連記事のスコアリング
- ソート（newest / popular / read_time）
- ページネーション
土曜日午前中に「これを綺麗に分けよう」と決めた。最初の作業は PR 5 個に分割する設計。
PR 設計
最終形（コード）から逆算して、5 つの PR に分けた:
| # | タイトル | 変更ファイル | 行数 |
|---|---|---|---|
| 1 | refactor: extract Posts::QueryParser | 2 | +60 / -20 |
| 2 | refactor: extract Posts::FilterApplier | 3 | +85 / -45 |
| 3 | refactor: extract Posts::SortApplier | 2 | +40 / -30 |
| 4 | refactor: extract Posts::Paginator | 2 | +35 / -25 |
| 5 | refactor: SearchService becomes a thin orchestrator | 4 | +30 / -55 |
各 PR は テストが通る独立した状態 で完結する。順番にマージしていく。
なぜ 5 個に割るのか
3 つの理由:
1. レビューの認知負荷を下げる
1 PR が 80 行未満だと、5 分でレビューできる。レビュアー（私が自分自身でも）が「全体を 1 度に理解する」必要がない。
2. ロールバック粒度を小さくする
1 つの PR で問題が出たら その PR だけ revert できる。8 時間分の作業を全部失わずに済む。
3. 区切りで止まれる
5 個の PR を すべて同じ日に書く必要はない。途中で疲れたら 3 個までで止めて、残りは翌週にできる。完了の単位を小さくすると、休日の体力に合わせて調整できる。
進め方
1 日の作業ログ:
- 9 PR 1 着手 / 10 PR 1 マージ
- 10 PR 2 着手 / 11 PR 2 マージ
- 11 昼食
- 13 PR 3 着手 / 14 PR 3 マージ
- 14 PR 4 着手 / 14 PR 4 マージ
- 14 PR 5 着手 / 16 PR 5 マージ
休憩込みで 7 時間。これを 1 つの巨大 PR で進めていたら 12 時間 はかかったはず（途中でデバッグループに入る確率が高い）。
失敗から学んだこと
最初の試みでは「5 個に分けたつもりが、PR 1 → PR 2 でテストが落ちる」が起きた。原因は PR 1 の段階で SearchService が QueryParser を使う形に変えた ため、PR 2 でさらに FilterApplier を抽出する変更がコンフリクトを起こした。
教訓: 各 PR で SearchService の API は変えない。新しいクラスを抽出して、SearchService 内部で使い始めるが、外部からは同じ API。最後の PR 5 で SearchService 自体を整える。
# PR 1〜4 では SearchService の外部 API は変えない
class Posts::SearchService
def call
# 段階的に内部実装を新クラスに置き換え
parsed = Posts::QueryParser.new(@query).call # PR 1 で追加
filtered = Posts::FilterApplier.new(...).call # PR 2 で追加
sorted = Posts::SortApplier.new(...).call # PR 3 で追加
paged = Posts::Paginator.new(...).call # PR 4 で追加
paged
end
end
PR 5 で call を一気にきれいにする:
def call
pipeline = Posts::QueryParser >> Posts::FilterApplier >> Posts::SortApplier >> Posts::Paginator
pipeline.call(query: @query)
end
計測
PR 分割によって変わった指標:
- レビュー時間: 1 個 5〜15 分 / 全体 50 分（巨大 PR なら 90 分以上）
- revert 確率: 0%（1 PR が独立しているので問題があれば部分 revert）
- CI 時間: 各 PR で 8 分 × 5 = 40 分 / 巨大 PR なら 25 分（CI コストは増える）
CI コストは増えるが、レビュー / マージの心理的負荷が劇的に下がるので、トータルで効率は高い。
まとめ
リファクタリング 1 日分を PR 5 個に分割するパターン:
- 最終形から逆算して PR の段階を設計 する
- PR 1〜N-1 は 外部 API を変えない
- 各 PR は テストが通る独立した状態
- 1 個 80 行未満を目安に切る
- 最後の PR で全体を整える
巨大 PR を作りそうになったら 「これを 5 個に割れないか？」 を最初に問う。割れなければ、設計が悪い。
リファクタリングはサイズではなく刻みで進める。1 歩ずつ main にマージできるか。