プルリク運用の覚書
プロダクトに対する変更は全てプルリクで行う
- masterブランチへ直接pushみたいなことはしない
- 軽微な修正や緊急対応でもこれは厳守すること
- 緊急時はmasterブランチへ直接変更をコミットしたくなる誘惑に駆られるのは理解できるが、プルリクを作る時間を節約したところでどのみち大したことはできないので、一旦冷静になるという意味でもプルリクを作った方がよい
プルリクのフォーマット
- プルリクは↓のような情報を載せておくとレビュアーに優しい
# 概要
- 概要を1行くらいで書く
- 課題管理ツールのリンクもここに
# 変更内容
- 変更内容を箇条書きで記述する
# 動作確認方法
- 特定のデータセットが必要だったり、環境の事前準備が必要な場合は記述する
# レビュアーへの依頼事項
- 何を重点にレビューして欲しいか
- レビューする必要のないことがあればそれも記述
- 逆にやったことがリストアップされているだけでは、レビュアーには意味のある情報にはならない事に注意
プルリクのレビュー
- 原則として、レビュアーはプルリクの差分だけでなく全てのコードを見ることを推奨する
- 差分だけでは修正のヌケモレがないか判断しづらいことが多い
- ただし、差分だけで判断できるような簡単な修正はこの限りではない
- 全体を何度も目を通すことでプロダクトコードに対する理解が進むことも期待できる
ダメなPR
- 複数の機能が含まれている
- 「ついでに」行った作業が含まれている
- 差分が膨大
プルリクはひとつの課題だけを扱うこと
- リファクタリングと機能追加を混在させないということ
- シンプルにレビューがつらい
プルリクの分類
追加
削除
リリース
プルリクの差分の大きさは最大100行を目安にする
- あまりにも大きな差分はレビューが困難
- レビューが困難であるということはレビューが雑になる原因になる
- レビューが雑になると当然不具合も混入しやすくなるので、良いことがひとつもない
- したがって、プルリクの差分は100行程度を目安にし、100行を超える場合はプルリクのテーマを分割できないか検討する
- とはいえ、一度作ってしまったプルリクを作り直すのは作業的にも精神的にも厳しいものがあるのは事実なので、潔く謝ってレビューを依頼する選択も許容されるべき（何度も繰り返すのは論外だが）
プルリクの寿命は短くする
- 何週間もレビューのやりとりが続くようなプルリクは作らない
- 原則としてプルリクはシングルイシューで作成し、こまめにマージしていく
- 事情がありmasterブランチにマージできない場合はリリースブランチを作り、そちらにマージしていく
- 「リリースまでマージを待ちたい時はリリースブランチを切る」を参照
プルリクのテーマとは関係ない問題は課題管理システムへ
- 潜在的不具合など、プルリクのテーマとは関係ない問題を発見してしまった場合は同一プルリクの中でついでに修正するのではなく、一旦課題管理ツールに記録するに留める
- プルリクのテーマから外れた指摘と修正を受け入れるとプルリクは際限なく巨大化することになり、それはプルリク運用として有益ではない
- タスク管理上もひとつのタスクがダラダラ長引くことは望ましくない
- したがって、プルリクのテーマとは関係ない問題は課題管理ツールに記録し、別途改めてその問題に対応するプルリクを作成する
承認されたプルリクを変更しない
- 一度承認されたプルリクは極軽微な修正（typoなど）を除き、原則として変更してはいけない
- したがって、プルリクが承認されたらさっさとマージする
- 承認後にプルリクが大きく変化すると「私が承認したコードはそれ（変更後のコード）じゃないんですが？？？」問題が起き、レビューの意味が無くなってしまう
- 機能追加が中途半端などの理由でmasterブランチにマージできない場合はリリースブランチに集約する
リリースまでマージを待ちたい時はリリースブランチを切る
- リリースブランチからタスクブランチを切ってこまめにプルリクを作る
- 最終的にリリースブランチのレビューを行い、メインブランチにマージ
- リリースブランチとタスクブランチの管理はそのブランチのオーナーの責任で行う（メインブランチの変更の取り込み、タスクブランチの親子関係など）
新機能の追加でも段階を踏んでマージしていく
- 新機能の追加は機能が完成するまで時間がかかる
- その為、プルリクの寿命も伸びる
- しかし、寿命の長いプルリクはやはりツラい
- そこで、新機能の追加時も段階的にプルリクをマージしていく戦略を採ることを検討する
- 具体的な手順は↓のような感じ
- (1) 空実装のモジュールを追加し、プルリクを作成する
- レビューではモジュール配置や設計についてレビューを行う
- レビューが完了したらmaster（もしくはリリースブランチ）にマージする
- (2) 空のモジュールにクラスや関数を定義し、プルリクを作成する
- この時点では具体的な処理は記述せず、コメントで実装方針を記述する
- レビューではインターフェイス（クラスのパブリックメソッド、公開関数）についてレビューする
- レビューが完了したらmaster（もしくはリリースブランチ）にマージする
- (3) 具体的な処理を実装する
- この段階から具体的な実装を記述していく
- モジュール構成やインターフェイスの設計は完了しているので、この時点で悩むことはあまりないはず
- ユニットテスト、動作確認が済んだらプルリクを作成
- (4) リリースブランチのプルリク作成
- リリースブランチを作って作業している場合、作業の締めとしてリリースブランチをmasterブランチにマージする為のプルリクを作成する
- レビュアーはリリースブランチのコードがmasterブランチにマージしてもよい品質であるかを中心にレビューを行う
- リリースブランチは
git rebaseでコミットをひとつにまとめておくとgitのログが綺麗になる
フィーチャーフラグを利用した分割リリース
参考資料