小さく分割する
なぜCLを小さくすべきか?
- レビューが速い。
- もっと丁寧なレビューを受けることができる。
- バグが発生する可能性が低い。
- 拒絶された時に無駄になる時間が少ない。
- コードをマージしやすい。
- きちんと設計しやすい。
- レビューに頼らず、他の作業をすることができる。
- 問題が生じた時に元に戻しやすい。
レビュアーとしては、CLが大きすぎるという理由だけでCLを拒否できる。そうなると、すでに巨大になったCLを分割する作業は大変で時間もかかるので、最初から小さなCL単位で開発する方がはるかに簡単かつ安全である。
どれくらいが小さなものか?
CLには、1つの独立した変化だけがあるようにする。最低限で、1つだけを含める。機能(feature)の一部のみを1つのCLにする。ただ、レビュアーが知る必要がある全ての情報を含む。もちろん、CLがマージされた後でも、プログラムは問題なく起動されなければならない。小さすぎてもいけない。新しいAPIを追加した場合は、そのAPIを使用するコードも同じCLに作成する。
絶対的な基準はない。経験上100行程度なら適切だと思う。1000行は大きすぎる。また、単一のファイルで200行を修正するのは大丈夫だが、同じ量でも50個のファイルに分散しているのは良くない。自分が大丈夫だと思うサイズより少し小さくすると、レビュアーが文句を言うことはほとんど無い。
巨大なCLはいつ許可されるか?
ファイル削除は1行の修正で見てもよい。ツールによって自動生成された長いコードも同様です。1行ずつ見る必要なく、ファイルの使用有無だけ決めれば良いからだ。実際のレビュアーを基準にファイルを分割してCLを作成する方法も良い。CLを2つに分けて、それぞれ他の人にレビュー要求をしてお互いを参照できるように説明文に言及しておく。
リファクタリング作業は分離する
リファクタリング作業は、機能やバグ修正から分離する必要があります。クラス名を変更したり位置を変更する作業を1つにまとめ、クラス内部のバグ修正は別のCLにする。ローカル変数名ぐらいはバグ修正と同時に変更しても構わないが、最終的にはレビュアーと作成者が議論して決定する。
対応するテストコードは同じCLに含める
一般的には、本番コードとテストコードを1つのCLで構成する。ただ、上記のリファクタリングのようにテストコードの修正だけを先に反映する場合もある。例えば、既存のコードに新しいテストを追加する場合、テストコードをリファクタリングする場合、統合テスト(integration test)を追加する場合などがある。
ビルドを壊さない
互いに関連する複数のCLを同時に作業している場合は、各CLが別々にコードに反映されてもビルドが壊れないようにする。そうでなければ、同僚の開発者のビルドも一緒に壊れることになる。
小さいほど良い
CLをこれ以上小さくすることはできないと思っても、本当にそのような場合は非常に稀である。CLを小さくすることを練習する開発者は、常に小さくする方法を見つけることができる。最初から小さくする自信がなければ、まず仲間たちと話し合い、アイデアを得る。それでも方法がない場合は、あらかじめレビュアーの同意を得て、自分もより鋭い目でバグを探し、より丁寧にテストを作成する。