この記事で書きたいこと
「commitの粒度ってどうすればいいの?」という質問に対して考えたことを書こうと思います。以前は「意味ある単位でコミットしよう」などとしか言えなかったのですが、Gitを何年か使ってきた中で、なんとなく、こういう時はこういう単位だと嬉しいなと言語化できる部分が増えてきました。私の経験からの記事になります。一人の意見として参考になればと思います。
どんな人向けの記事なのか
「Gitは普段使うのに困らない。commitは意味のある単位で細かくしてね、とは言われるけど、具体的にどんな単位?」と思っている方向けの記事になります。Gitのコマンドの説明などは行いません。
どんな単位が良いのか
「この単位が良い!」というよりは、いくつか検討するポイントがある、といった感じです。具体的には以下のポイントです。
エラーがなく動く単位でコミットする
コミットする時点で実際に動かしてみたときにエラーが発生しない、というのは一つのチェックポイントになると思います。
エラーが起きないと何がいいのか
主に障害対応でコミットを1つずつ遡って検証する際に、動かないコミットがあるとそこで検証がストップしてしまいます。そのエラーが「中途半端な状態でコミットしたがゆえに起こっているエラー」とわかれば良いのですが、わからないことが多いと思います(そのコミットを行った本人であればいいかもしれませんが、本人であっても忘れていることも当然あります)。実際には、それがなんのエラーなのか調べることになり、本来の検証を進めることができなくなってしまいます。
静的解析、単体テストも含められるとより良い
静的解析の修正結果や単体テストの修正も、コード修正のコミットに含められると良いと思います。これも上記と同じような理由になります。そのコミットに戻してそこから作業したい、といった場合、静的解析でエラーが発生する、単体テストが動かない、となってしまうと、そのコミットのコードはOKであってもそのコミットに戻す+αの作業が発生します。
また、特に単体テストの改修がコミットに含まれていれば、その修正に対するテストが行われていること、どのような観点でのテストが行われているのかがレビュアーにも伝わりやすくなると思います。
エラーが起きなければコミットが大きくなっても良いのか
ここが難しいところだと思いますし、チームや人によっても違うと思います。このあたりがよく言われる「意味のある単位でコミットしよう」ということだと思います。
例えば以下の3つに変更を加えるタスクがあるとします(なんとなく雰囲気で捉えていただけると嬉しいです・・・)。
- UI
- UIから呼び出す関数
- その関数が使っているサービス
この場合、私だったら、
UI + UIから呼び出す関数 + これらにまつわる単体テスト + 静的解析で1コミット、その関数が使っているサービス + 単体テスト + 静的解析で1コミット、の2コミットに分けます。単体テストの単位がこんな感じになりそうだな、というのと、サービスはUIに依存せずに独立したものになるはずなのでコミットとしても分けています。
後で見たときになんでこの変更を入れたのかがわかる単位
「なんでこんなコードなんだっけ?」というときに、 git blame
してコミットIDを調べ、 git show <commit-id>
でそのコミット全体を見てみる、といったことをする場合が私はあります。そのときに適切なコミットメッセージ(Bug fixとか指摘対応とかではなく)が入っており、そのメッセージにあっている変更であれば、作業が捗ります。ここについては「具体的にこういう単位」とまだまだ言語化できていないのですが、自分がコミットIDからコミットを探すような行動をとることがあった場合に考えてみると、後の自分のコミットに役立つかもしれません。
###renameやファイル移動などの機械的な変更は別コミットにする
例えば先ほどの、UI〜サービスまで手を入れる場合に、サービスの関数名をより良いものに変更しようという場合があると思います。もしくはファイル名を変えたり、ファイルの階層を移動するといったことがあるかもしれません。このような場合は、実際のサービスの修正とは別に、renameやファイル移動の変更単体で(静的解析や単体テスト込みで)コミットします。
renameやファイル移動でコミットを分けると何が嬉しいのか
レビューがしやすくなります。レビューをする場合に、コミットごとに変更内容を見ていくことでレビューがしやすくなります。そのときに、renameやファイル移動のコミットがわかれていると、本来レビューしたいロジックの変更などに着目しやすくなります。renameやファイル移動に関する修正点って意外とありますよね。その修正点を別にしておけるのはとてもメリットがあると思います。
また、renameやファイル移動を万が一ミスしてしまっている場合、コンパイル言語であればコンパイルの時点でエラーが出るでしょうし、そうでなくても実際に動かせばエラーになると思います。そのため、どんな名前に変更したのか、については興味があっても、renameの漏れやimport文の修正はさらっと確認するレベルで良いと私は思っています。この点からも、コミットを別にする利点はあると思います。
###レビューの指摘対応は指摘ごとにコミット
レビューで指摘を受けて修正する場合、指摘ごとにコミットをするようにするとよいと思います。そうすることで、指摘に対してどのように対応したのかをレビュアーが確認しやすくなります。そして、指摘の意図を理解して、どういう意図で変更したのかをコミットメッセージに入れると良いと思います(コミットメッセージに「レビュー指摘対応」とだけ入れてしまうと、後でコミットを見直すときに何が目的でこの変更を入れたのかがわからなくなってしまいます)。
とはいえ、完全に指摘ごとにコミットを分けてしまうと、指摘コメント=コミットの数が増えるということで細かくなりすぎる場合もあります。指摘の理由が同じであれば対応箇所が違う行やファイルであっても同じコミットにしてしまって良いと思います。
###タスクに関係がない修正は別コミットにする、もしくは別プルリクにする
ボーイスカウトは素晴らしい
タスクを行なっている中で、タスクには全然関係ない改善点を見つけることもよくあると思います。typoとか。コメントと実装の内容があっていないとか。気づいたときに直していくという行動はとても素晴らしいと思います。そして、コミットも別にしておくとさらに良いと思います。
やはりレビューがしやすい
これもrenameを別コミットにするのと同じ理由です。レビューする人からはレビューしやすくなります。また、万が一修正したコミットが間違っていた場合(そしてこのコミットにtypoの修正も含んでいた場合)、そのコミットをrevertするかもしれません。そうすると、間違っていた内容と一緒に「間違っていない修正(ここではtypo)」まで失われてしまいます。
コミットを分けてさらに別プルリクにするのもあり
今やっているタスクとは別のプルリクにするのもありだと思います。私であれば、typoはタスクのプルリクに含めてしまうことが多いです。コメントの間違えなどは、単純なミスのようなもの(その引数は存在していないとか、返り値の型が違うとか、typoとか)はタスクのプルリクに含めてしまいますが、「これは・・・なんか違う?」というようなちょっと検討が必要な場合(その関数の読み込みをしたいときなど)や、なぜこのコメントが間違っているのかを詳しく説明しなければならない場合(この場合はそもそもコメント変更だけで済まない場合もあると思いますが)は、一旦置いといて別プルリクにすることもあります。また(タスクに関係がない)renameやロジックの変更などは完全に別プルリクにしています。
タスクに関係がないものはどんな小さい変更であっても別プルリクにしましょう、というチームもあると思います。この辺はチームにとってやりやすい方法が変わってくると思います。
###コマンド実行による変更は1つのコミットにする
チームやプロジェクトによっては、よくある変更を簡単に行えるようスクリプト化しているといった場合や、コマンドを実行することによってコードに修正が入る、といったこともあると思います。そういう場合はそのコマンド(やスクリプト)を実行した結果を1つのコミットにして、コミットメッセージには実行したコマンド(やスクリプト)を入れておくと良いと思います。同じチームのメンバーであれば「ああ、あれね」といった感じで伝わりやすいはずです。
###コミットメッセージに困るようであれば単位を見直すべきかもしれない
以前、私はこれがよくありました。小さい単位でコミットしようとしすぎて、コミットにメッセージをつけられない現象です。コミットメッセージにはなぜ(Why)を書くと良いと言われています(ここでは説明を省きます)。思いつかない場合は、もしかするとコミットしようとしている粒度が小さすぎて意味をなしていないのかもしれません。もう少し実装を進めてみる、もしくは一度コミットしておいて後で見直してみると良いかもしれません。
以上5つのポイントについて説明しました。
##コミットしてはいけない、ということではない
コミットの粒度などについて考え始めると、なかなかコミットができなくなる、といったことが起こるかもしれません。ですが、それでコミット漏れがあったり、pushできないまま翌日休むことになり誰にも作業を引き継げなかった、などが起こると本末転倒です。コミットの粒度を考えられるというのは、作業の段取りができていたり、作業の見通しが立っている、ということが前提にあるとも思っているので、明日すぐできるのかというと難しい部分があると思います。
慣れてくれば意識せずとも粒度を考えてコミットできるようになると思います。コミットは後で修正することも可能ですし(この後説明します)、最初から完璧に行わなければ開発が進まないといったものでもありません。ちょっとずつ意識しながらよりよくしていく精神で、緩くいくと良いと思います。
便利なコマンド
このようにコミットの粒度について考え始めると、一度したコミットを修正したい場合が出てくると思います。そんなときに便利なコマンドです。ここではコマンドの紹介に留め、使い方については省略させていただきます。
rebase
git rebase -i <commit-id>
HEADから commit-id
までのコミットをまとめたり消したり順番を変えたりコミットメッセージの編集をしたり、などできます。「今日はも業務終了だからできたところまでpushしたい」といって作業途中でコミットした場合や、静的解析忘れていた!という場合など、後から修正することができます。とても便利です。
rebase は既存のコミットを修正してしまいます。revert コマンドでコミットを消す場合は「コミットを消した」という記録がコミットとして残りますが、 rebase はそのような記録が残らずコミット自体を書き換えてしまいます。そのため、他のメンバーと共有しているリモートブランチの内容を勝手に rebase で書き換えたり、開発の最新が詰まっているブランチやましてやmasterやmainに対しての rebase はNGです。チームの運用方法によると思いますのでご注意ください。
##最後に
最後になりますが、コミットの粒度やコミットメッセージなど、凝りすぎるとミスります。お気をつけください。
また、記事の中でも何度か書いておりますが、これが正解、というわけではありません。開発スタイルやチームのメンバーによって、何がチームにとってプラスになるのかは変わってきます。
一つの案として、悩んでいる方の参考になれば幸いです。