PRを小さくする勉強会

PRを小さくする勉強会
分割統治で開発を制する

Transcript

1. PRを小さくする勉強会 分割統治で開発を制する NCDC株式会社　武方順平 対象: 社内エンジニア 時間: 60分 PRを小さくする勉強会 分割統治で開発を制する 1
2. こんなことないですか？ PRを小さくする勉強会 分割統治で開発を制する 2
3. 「大きいPRはレビュー大変なので、分けて小さくしましょう！」 「わかりました！（でも、どうやって？） 」 PRを小さくする勉強会 分割統治で開発を制する 3
4. 今日のゴール レビュイー：観点に沿ってPRを設計・分割できるようになる レビュアー：観点を指定して具体的なレビュー指摘ができるようになる PRを小さくする勉強会 分割統治で開発を制する 4
5. 今日のアジェンダ 1. 導入（10分） - なぜPRを小さくするのか？ 2. 分割統治（10分） - 分ける観点の全体像 3. 各観点の実践（25分） - 具体的にそれぞれの分け方を紹介する 4. ケーススタディ・討論（15分） - 実践に向けて PRを小さくする勉強会 分割統治で開発を制する 5
6. 1. 導入 そもそも、なぜPRを小さくするのか？ PRを小さくする勉強会 分割統治で開発を制する 6
7. PRを小さくする理由 観点 問題 小さくすることで得られる効果 ① 認知負荷の観点（レビューの しやすさ） 大きいPRはレビュアーが理解しきれず、 表層確認に終わる 小さいPRは1回の集中で理解可能 → 指摘が深く、品質向上 ② フィードバックループの観点 （リードタイムの短縮） 大きいPRほどレビュー待機・修正・再レ ビューで遅延 小さく分ければ部分ごとに並列処 理・早期マージが可能 ③ リスク制御の観点（変更影 響・障害対応） 変更範囲が広いとCI失敗・リリース後のバ グ特定・ロールバックが難しい 狭い変更は原因が特定しやすく、 rollbackも容易 → "速度・品質・安全性" の3軸すべてに直結する。 PRを小さくする勉強会 分割統治で開発を制する 7
8. 実データで見る現状 9月某週の弊社データ（全repository横断） →レビュー出してからapproveまで一週間かかっている →レビュイーからすると待ち時間がボトルネックになっている PRを小さくする勉強会 分割統治で開発を制する 8
9. PRを小さくすることの期待効果 1. レビュー着手の改善 小さなPRは「すぐ終わる」認識で優先されやすい 2. レビューサイクルの効率化 指摘減少、影響範囲明確で修正・再レビュー時間短縮 小さな変更範囲 → レビュアーが集中しやすい → 見落とし減少 3. レビュー品質の向上 理解しやすい → より深いレビューが可能 バグの早期発見 → 後工程での修正コスト削減 4. マージプロセスの効率化 既存影響なしでオートマージ設定も可能になりうる →　各段階で20-30%の効率化ができれば、大幅なリードタイム短縮が期待できる PRを小さくする勉強会 分割統治で開発を制する 9
10. 批判と反論 批判 反論 PRを小さくすると作業が増えるた め、生産性が下がる 一時的に作業数は増えるが、待機時間が圧倒的に 減るため総リードタイムは短縮 動くことを確認していないと途中 段階マージできない 段階的実装・Feature flag・モック活用で“動く途中 段階”を成立させることが可能 PRを小さくする勉強会 分割統治で開発を制する 10
11. なぜ小さくするのかのまとめ PRを小さくするのは、開発を速くするためだけではなく、 変更の影響を制御し、確実に理解しながら進めるため。 PRを小さくする勉強会 分割統治で開発を制する 11
12. 2. 基本編 分割統治で開発を制する PRを小さくする勉強会 分割統治で開発を制する 12
13. どうやってPRを小さくするのか？ 作る機能が同じなら、コードの変更はだいたい同じ コードの再利用などでも小さくはなるが、限界がある → 分けることでトータルの変更量は同じでもPRを小さくすることができる PRを小さくする勉強会 分割統治で開発を制する 13
14. でも、結局トータルで同じコード量変更するならレビュー回数増えるだけ損で は？ “小さく分けてもトータルの変更量は同じ”というのは正しい。 しかし、レビュー待ち・調整待ちなどの「非作業時間」こそが最大のロス。 小さく分けることでこのブロッキング時間が減る。 PRを小さくする勉強会 分割統治で開発を制する 14
15. 分割統治とは 課題を小さな部分に分けて、それぞれを解決してから統合する基本的な問題解決ア プローチ プログラミング一般・課題解決一般に通じる考え方 → PRでもこれをやる PRを小さくする勉強会 分割統治で開発を制する 15
16. 分割の観点 PRを分割するときの観点について考えてみる 観点 典型的な分割単位 メッセージ ①関心の分離で分ける 機能 vs リファクタ vs 設定変更 1PR＝1関心事 ②構造で分ける モジュール、レイヤー、DB変更段階 レイヤーやデータの境界で分ける ③ 状態・リスクで分ける 段階的実装、段階的マージ、Feature Flag 動く途中段階でもマージできるように ④ 制約で分ける マージ=リリース、有無・バージョニング制約 環境制約に合わせた安全な切り方 ⑤ チームで分ける チーム規模・レビュー体制・テスト責任 組織・運用単位で分ける PRを小さくする勉強会 分割統治で開発を制する 16
17. 3. 各観点の実践 具体的にそれぞれの分け方を紹介する PRを小さくする勉強会 分割統治で開発を制する 17
18. ①関心の分離で分ける 変更したい意図を整理して分割する 機能追加とリファクタリングは分ける バグ修正と新機能は分ける 設定変更とロジック変更は分ける PRを小さくする勉強会 分割統治で開発を制する 18
19. bad 実装中にリファクタリングの必要性に気がつくのでその場で行う リファクタリングを別作業ブランチにコミットして、機能完成と同時にPRに出す →開発しながらリファクタリングをしていくのはいいことだが、リファクタ+他の変更だと観点が増 えてしまう good 別ブランチでリファクタだけしてPRを出しておく →リファクタだけだと、動作に影響しないことがわかっているので、レビュアーもレビューしやすい 例 refactor: extract UserRepository interface レビュアー: 「こっちはリファクタだから設計観点でのレビューだけでいいな」 feat: add login usecase skeleton レビュアー: 「これは機能変更だから動作が担保されているか確認しなきゃ」 PRを小さくする勉強会 分割統治で開発を制する 19
20. ②構造で分ける モジュールや層・データ構造単位で分ける bad ユーザー登録APIを実装するときに、DB定義、データアクセス、ユースケース、コントローラーをまとめて実装して動くものを1つのPRにする good 各レイヤーを別のPRとして分割する 例：ユーザー登録機能の場合 1. PR1: User entityとDB migration追加 2. PR2: UserRepository実装 + ユニットテスト 3. PR3: RegisterUserUseCase実装 + ユニットテスト 4. PR4: POST /users エンドポイント実装 + テスト 各レイヤーの動作はユニットテストで担保し、統合テストは最終段階で実施 PRを小さくする勉強会 分割統治で開発を制する 20
21. よくある疑問 Q: DBだけやって、UseCase実装時にDB変更が必要になったら？ A: また1に戻ればいい PRが小さいので修正コストも小さい 早期に設計ミスに気づける レイヤー間の依存関係が明確になる PRを小さくする勉強会 分割統治で開発を制する 21
22. ③状態・リスクで分ける 手法 状態・リスクの分離としての説明 段階的実装 「まだ使われない状態のコード」を先にマージし、リスクを段階的に限定す る。コードは先行、機能は遅行。 Feature flag 機能を“論理的に有効化／無効化”して、リリースタイミングを制御する。PRは 安全に積み上げられる。 互換性維持（バージョニングなし 対応など） 旧仕様を残しつつ新仕様を導入し、リスクを並列化して吸収する。 PRを小さくする勉強会 分割統治で開発を制する 22
23. bad 新機能を完全に実装し、既存機能との統合も含めて全て動く状態にしてから1つのPR で出す 新機能を直接マージして本番リリース good コードをマージしてもまだ使われない状態を作る（段階的実装） 環境変数などで機能フラグを有効にしなければ新しいコードを通らないようにする ex. 新機能を無効状態でマージ 段階的に有効化 問題があれば即座に無効化可能 PRを小さくする勉強会 分割統治で開発を制する 23
24. ④制約で分ける 制約で分けるとは、 「理想的な順序・分割が取れない状況」でも、安全に進めるための考 え方。 たとえば「マージ＝リリース」 「APIバージョニングがない」など、 環境・プロセスの制約を前提にした現実的な分割戦略を指す。 PRを小さくする勉強会 分割統治で開発を制する 24
25. 例1. ダウンタイムなしのDB変更 既存カラムを変更したいが、サービス停止できない制約がある場合 安全な変更順序: 1. PR1: 新カラム追加（NULL許可） 2. PR2: アプリケーション側で新カラムに書き込み開始 3. PR3: 既存データを新カラムへ移行 4. PR4: 新カラムにNOT NULL制約追加 5. PR5: 古いカラム削除 各PRは本番環境で動作しながら段階的に進められる PRを小さくする勉強会 分割統治で開発を制する 25
26. 例2. APIバージョニングなしの場合 新フィールド追加→クライアント適用→旧フィールド非推奨→削除 受入側は後方互換（旧形式も受理）を先に入れてから、出力側を切り替える PRを小さくする勉強会 分割統治で開発を制する 26
27. 例3. マージとリリースの関係の影響 原則：Feature Flag必須／未使用コードOK／UI非表示 “検証を切る”設計：if flag: 新ルート else: 旧ルート を先に入れる 本番影響を切る：ダークローンチ（裏で処理、ユーザー未露出） マージ=リリースの場合 より慎重にPRを分割する必要 feature flagが必須 不完全な状態でもマージ可能にする設計 マージ≠リリースの場合 より大胆にPRを分割できる developブランチで統合テスト リリース時にまとめて有効化 PRを小さくする勉強会 分割統治で開発を制する 27
28. ⑤プロセスで分ける 技術的な依存関係ではなく、人的・運用的な依存関係を断ち切るための分割です。 つまり「誰が、いつ、どの責任で進めるか」というワークフロー単位でPRを分ける、 → 複数人の承認がないとレビューが進まない、ということを避けられる PRを小さくする勉強会 分割統治で開発を制する 28
29. 例1：レビュー責任の分離 仕様設計やAPI契約部分 → アーキテクトやPMがレビュー 実装PR → 担当エンジニアがレビュー 仕様確定と同時にコードが進められるように、PRを2段階に分ける レビュー待ちのボトルネックを解消し、並行作業が可能になる。 PRを小さくする勉強会 分割統治で開発を制する 29
30. 例2：CI／テスト段階で分ける PR-A：静的解析・ユニットテストまで通す PR-B：結合テストやE2Eテスト用の修正（別環境で動作確認） テスト環境やCI時間の制約を「プロセス」で分離する。 PRを小さくする勉強会 分割統治で開発を制する 30
31. 例3：担当者や専門領域で分ける DBマイグレーションはDB担当 API契約PRはフロントエンド代表が一次レビュー UI変更は別PRでデザイナーが確認 専門性と責任の明確化。チーム内で「どこまで見ればいいか」が明確になる。 PRを小さくする勉強会 分割統治で開発を制する 31
32. 4. ケーススタディ PRを小さくする勉強会 分割統治で開発を制する 32
33. 学んだ観点を使ってみよう ①関心の分離 / ②構造 / ③状態・リスク / ④制約 / ⑤プロセス 実践では複数の観点を組み合わせることが多い 例：構造で分けつつ、状態・リスクにも配慮する PRを小さくする勉強会 分割統治で開発を制する 33
34. ケーススタディ1: ユーザー認証機能の追加 従来のアプローチ 「ユーザー認証機能」で1PR 分割案（②構造 + ③状態・リスク） 1. 認証用データベーステーブル追加 ②構造（DB層） 2. JWTライブラリ導入と基本設定 ①関心の分離（設定） 3. ログインAPI実装（未接続状態） ③状態・リスク（段階的実装） 4. 認証ミドルウェア実装 ②構造（ミドルウェア層） 5. 既存エンドポイントに認証適用 ②構造（既存への適用） 6. フロントエンド側のログイン画面 ②構造（UI層） PRを小さくする勉強会 分割統治で開発を制する 34
35. ケーススタディ2: 既存APIの仕様変更 課題 レスポンス形式を変更したいが、APIバージョニングなし 分割案（④制約 + ③状態・リスク） 1. 新しいオプショナルフィールド追加 ④制約（後方互換性維持） 2. クライアント側で新フィールド対応 ②構造（クライアント側） 3. 古いフィールドを非推奨マーク ③状態・リスク（段階的移行） 4. 古いフィールド削除 ③状態・リスク（段階的移行） PRを小さくする勉強会 分割統治で開発を制する 35
36. ケーススタディ3: 実プロジェクトの事例 A社での実例 PR355 状況: 現場一覧取得API実装 変更量: +1863/-635行（実質1000行超） 開発初期で基盤未整備 テストコード含めて一気に作成 format等の無関係な差分も含む PRを小さくする勉強会 分割統治で開発を制する 36
37. 主な変更内容 1. 基盤整備（format、lint設定） 2. Machine entity削除（別タスク？） 3. Site entity修正 4. SiteRepository実装 + テスト（589行） 5. SiteUseCase実装 6. APIルート + バリデーション + テスト（215行） 7. ドキュメント更新 PRを小さくする勉強会 分割統治で開発を制する 37
38. 分割案（どの観点を使ったか） 使用した観点: ①関心の分離 + ②構造 PR 内容 行数 観点 1 基盤整備 50-100行 ①関心の分離 2 Machine entity削除 100-200行 ①関心の分離 3 Site entity修正 100-200行 ②構造（Entity層） 4 SiteRepository実装 + テスト 300-400行 ②構造（Repository層） 5 SiteUseCase実装 50-100行 ②構造（UseCase層） 6 APIルート実装 + テスト 200-300行 ②構造（API層） 7 ドキュメント更新 50-100行 ①関心の分離 PRを小さくする勉強会 分割統治で開発を制する 38
39. レイヤー分割の順番 トップダウン（API → UseCase → Repository） メリット: API仕様が先に決まる、フロント並行作業可能 デメリット: 下層の実装で仕様変更リスク ボトムアップ（Repository → UseCase → API） メリット: データ層の制約を先に理解、確実に積み上げ デメリット: 全体像が見えにくい PRを小さくする勉強会 分割統治で開発を制する 39
40. 使い分けのポイント 状況 推奨アプローチ 開発初期・仕様不確定 トップダウン データ構造が複雑 ボトムアップ フロント並行開発 トップダウン 既存システム拡張 ボトムアップ PRを小さくする勉強会 分割統治で開発を制する 40
41. この事例の改善結果 Wiki「PRの心構え」作成 週次Bot配信で啓発 現在は差分500行以下に改善 PRを小さくする勉強会 分割統治で開発を制する 41
42. グループ討論（5分） 各自の現場での分割困難なケースを共有 今日学んだ手法での解決案を検討 PRを小さくする勉強会 分割統治で開発を制する 42
43. まとめ PRを小さくする勉強会 分割統治で開発を制する 43
44. まとめ 分割統治の適用: 大きな変更も小さく分けて段階的に 関心事の分離: 機能・技術・タイミングで分ける 現実的な制約: チーム・プロダクトの状況に応じて調整 投資対効果: 短期コスト増だが長期的にはメリット大 PRを小さくする勉強会 分割統治で開発を制する 44
45. ご清聴ありがとうございました 質問・ディスカッションタイム PRを小さくする勉強会 分割統治で開発を制する 45