git commit の質について

先日仕事先で git の commit や pull-request の出し方について有益な議論をすることができたため、簡単に残しておく。

以下 GitHub の pull-request (GitLab を使っている場合は Merge Request に相当) を p-r と表記する。

レビューしやすい pull-request (commit の粒度)

個人的には以下の条件を満たしていれば大体レビューしやすくなっていると思う。

  • commit 数が少ない
  • 1 commit あたりの変更が多くない
  • 各 commit がほどほどに独立している(疎結合)

コードレビューの優先順位が低くなってしまっている組織は経験上少なくないと思うが、こういう点を各自が気を付けることでコードレビューをする敷居を下げられるのではないかと思う。

commit 数が少ない

例えば email + 各種 SNS でのログイン機能 を実装するケースで考える。

  • 全部実装するつもりで login ブランチを切って作業する
  • commit 数が 10個以上になりそう or 変更があまりにも大きくなった

のようなケースがあった場合、そのまま p-r を出してしまうとレビューする方は大変だし、修正する必要が出た場合に手間も増える。

手間が増える、というのは git commit --fixup で修正して最後に rebase するという対応をすることが前提である。 「指摘された点を修正」などという commit を残すのは論外なので。

そのため以下のような方法を取るとレビュワーに優しい p-r にすることができる。

  • email_login という branch を別途切り出して email ログイン機能だけ cherry-pick して p-r を出す

「この機能開発はどれくらいの粒度で branch を切ればいいか」の温度感は経験を積めば慣れると思うが、慣れない言語や FW を使っているとたまにこういうことは起こりうる。

1 commit あたりの変更が多くない

体感や個人差があると思うので必ずしも守れるものではないというのは断っておく。 ただ commit message の主語を大きくし過ぎないように気をつけてさえいればある程度は避けられると思う。

各 commit がほどほどに独立している (疎結合)

これは 1 branch あたりで commit 間に依存関係が出るのは不思議ではないので必ずしも成立しえないが、 例えば 1つの commit 内で「formatter による修正」と「機能追加」を行うと変更が見づらくなってしまうのでやめろ、ということはできる。

一つ一つのコミットで意識すべきこと

自分は先の議論の中で「そのコミットで何ができるようになったか を意識してみたらいいんじゃないか」と発言したが、 他の方からは「あとから切り出しやすいように commit してもらえると助かる」ということも言われて確かにそれは重要だなと感じた。

例えば HEAD~3 で行った変更を HEAD~1 で修正、みたいなのは git commit --fixupgit rebase -i などをうまく使って一つにまとめればいいし、 revertcherry-pick しやすいように log を綺麗にしておく、などといったことはチーム開発する上で重要なことだと感じた。

この 歴史の修正 (自分は違うと思うが)に関しては、別のチームで以前「歴史修正主義が云々」みたいな記事を紹介されたりして辟易したが、あくまで仕事で git ないしは何かしらの VCS を使っているなら「後から見返しやすい」ことや「切り出しやすい」ことは生産性に直結してくるので、そういった議論(?)を持ち出してくるのはせいぜいあなたの個人プロジェクトだけにしてほしいな、と感じたし、この記事を読んでいる人には控えてほしいと思う。

まとめ

気遣い大事


2020-01-09 更新: 別の記事と表現に一貫性がなかった点を修正, 説明が不十分だった点を追記


Contents