小さいPull Requestを作る工夫
社でPull Request（以下PR）を小さくする工夫の話が盛り上がっていて機運だったので書いてみる
自分は普段PRを作るときも見るときもPRサイズを小さくするようにしている
見るときというのは、レビューにアサインされたときの話で、PRのサイズが大きかったら分け方の方針の提案込みで分割するよう依頼している
そんな自分が考える、PRがでかくなってしまうケースとそれに対して小さくするアプローチ、というのを書き出してみようと思った次第。
なぜPRはデカくなるのか
小さいPRを作る工夫、とは言ってみたが、むしろなぜPRはでかくなるか、そこからの対策、というブレークダウンで考えてみることにする
- PRを小さくしようとしていない
- 一つのことを実現するのにデカくなってしまう性質の機能である
- サーバーとクライアント一緒くたに作った
- 一度の変更で複数のことをやっている
- テストを丁寧に書いた
- 依存関係的に1箇所変更すると修正箇所が波及してしまう
- 自動生成コードを含んでいる
これらに対してどういうアプローチで対処するかを考えていく。
そもそも小さくしようとしていない
意識的に小さくすることを避けている
それならそれで良いと思う
小さくすることを考えたことがない
まずは小さくすることを考えてみてもらうということでひとつ
小さくすることに心理的抵抗がある
小さくしてみようと頭では思っても実際にやるのは面倒・大変といった気持ちを持っているケース
自分が過去に辿ったのは↓みたいな思考だった。（他の人と話す中で同じような考えを持ってそうと思ったこともあったような気がする
- やってみたらでかくなった
- 作りきったしそこから分けるのは手間が多い
- そもそも小さくするのは難しい
- 設計をちゃんと考えるのはコストが高い（やってみないと分からないことが多いケースでよくある）
- かといって考えきらずに決め打ちで始めると手戻りがち
- それなら最初から一気通貫でやって動くものを作るのが手っ取り早い
このようなケースでは以下のような考え方・アプローチで対策している
→大きくなってしまっても構わないので、まず荒削りで作りきってしまい、そこから分割を考える
これは設計とかも固まってないなら泥臭く雑でもいいから正常系1個だけ最低限通すとか、とにかく製品の品質には足りないけどごく最低限動くレベルで作り、
それを元にして分割を考えるというやり方
不確実性が高い状況だとこういうアプローチのほうが結果的には速いことも多い（はず）
PRのサイズとは異なる文脈だけど、設計を明らかにするとか、考慮事項の洗い出し、規模感の把握といった点でも有効な方法なので、割とよくやる
このアプローチを取ると、既存のロジックにどう手を加えればいいかが明らかになり、事前に必要なリファクタリングや作るべきモジュールがはっきりするので、
その単位でコードを清書してPRを出していくとある程度整理された状態でPRを出していける
このアプローチは大掛かりな機能だと難しいけど、そういうケースはそもそももっと上流工程のテクニックが必要になると思う
一つのことを実現するのにデカくなってしまう性質の機能である
既存のモジュールがデカい・結合度を下げることもアーキテクチャ上困難・その上で影響範囲が広いケース
例としてTypeScriptの型システムに機能を追加する、みたいなの
多分わりとどうしようもないと思う
でも普通のwebアプリケーション開発の範囲でこういう機能・モジュールになることってそんな内容な気はするのであんまり考えたことがない
テスト充実させておくとかしかないんじゃないかなあ・・・
サーバーとクライアント一緒くたに作った
機能を一気通貫で作ったケース。（サーバーとクライアント、みたいなのはあくまでも一例）
よくある。わけよう。
こういうのはcommitごちゃまぜになってたとしても技術的な境界で分割すればそんなに大変じゃないはず
ここでいう技術的な境界は、例えばクライアントサイドとサーバーサイド、サーバー内なら、DBアクセス層・ドメイン層・プレゼンテーション層などのアーキテクチャのレイヤー、のようなビジネスドメインではなく利用している技術による境界を称したもの
サーバーとクライアントサイドを実装した、という場合なんかは素朴に依存関係の末端から順に分割するようなやり方で十分通用すると思う
みたいなの
アジャイルプラクティスの文脈で、よくこういう作り方をすると全部結合するまで動かない、というデメリットを挙げられるやり方だけど、
そもそも全部作っちゃってから分割するときにこの方法を取るならそのデメリットもあまりないのでまあ良いかなと
サーバーで非互換な変更を加えてクライアントも同じPRでそれに対応、みたいなケースもありえるかもしれないけどそれが許されるなら手間をかけてまで分割しないほうがいいのかもしれない
一度の変更で複数のことをやっている
この機能作るならライブラリアプデして〜リファクタして〜あとついでにバグってるやつ直して〜とかしちゃうやつ
よくある。わけよう。
こういうケースは大体適当なところでcommit作らずに書き進めてごちゃらせがちなんで、1commitの粒度をある程度小さくすることを習慣にするのがよい
（と言いつつ自分は静的型付け言語で書いてると変更に対して強気にいられるからってクソデカcommitになっちゃいがちなんだけど・・・
ともあれ、commitさえ変更がそれぞれ独立した形で分かれていればPRを分割するのは簡単なので、そのような習慣を作っていくと良いと思う
この話題の発端の同僚氏が書いてたのがちょうどそんな内容の記事 だったので張っておく
conventional commitにするというのは自分もやっていて、大体1commitでやることはconventional commitに分類できる粒度になるようにしている
（以下余談）
記事にはcommitはあんまりしないとあったけど、自分はマメにする派
自分は割と、「こうやればうまくいくんじゃなかろうか？」→（やってみる）→ダメだったので消す、というのを頻繁にやるので、commitはセーブポイントみたいに使いたい感じ
大体commitするときは「なんらかのキリのいい状態」（コンパイルが通るとかテストが通るとか）にしておきつつ、適当に変更してダメなら消すをカジュアルにやっている
テストを丁寧に書いた
テスト、プロダクションコードの変更に対して何倍ものコード量になるんで、ある程度仕方ない
これもまあしょうがないと思う
一個上の 一度に複数の変更をしない というのを心がけることでテストでのPRサイズ膨らむのを抑えられるので、そういう感じで頑張る
依存関係的に1箇所変更すると修正箇所が波及する
静的型付け言語・モノリシック・大きいコードベースなどでリファクタリングとかするとありがち
（静的型付けはそういうの言語機能で一発で出来ちゃうからどっちかというと やりがち 、のほうがニュアンスが近いかも）
そしてそういう状態そのものをどうにかする変更もデカくなるんでまあまあ踏ん張りがいる
こういうケースでは、変更内容がリネームやクラス・DBのテーブルへのフィールドの追加などのような単純な変更のみにするよう心がける
変更も前述した「一度の変更で複数のことをやっている」とのあわせ技で大きくなるケースもありがちなんで、単純な変更になるようにしつつ単独でPRを上げるのが効果的だと思う
（というか他にやりようあるのかな・・・
現実問題、単純な変更だけではいかないこともあると思う
その場合は可能な限り複雑さが局所的になるように設計して変更を加える、といった設計面でのアプローチで解決することになると思う
必要ならその準備のリファクタから入ったりしていくこともあるだろうが、まあ前提が影響範囲がデカいものなのでとにかくキツい・・・
そんな場合は歯を食いしばり両の腕のマッチョでシバいていくしかなさそう、辛い
自動生成コードを含んでいる
まあ割としょうがないんで、自動生成による変更だけのPRに分けましょう
commit分けておけばこの手の分割は簡単なのでやはりcommitは粒度を適当にしておこうということで
というわけでここまででPRがデカくなる理由からのPRを小さくするアプローチを書いてみた
紹介したアプローチを選ぶ指針とか、そもそも最初から小さくしようと思った場合のアプローチとかも書きたかったけどまとまりがなくなると思ったのでやめていおいた
機会があれば書くかもしれない（こういうの一生書かなそう）
