コミット粒度とコミット名で、レビュー時間は1/3になる
はじめに
「レビューお願いします！」
苦労して実装した機能、どんなフィードバックが返ってくるのか...とドキドキしているエンジニアのあなたに10秒後返ってくるコメントはこうです。
「中身まだちゃんと見てないけど、とりあえずコミット分けてください。」
頑張って書いたコードへのフィードバックはゼロ。
そんな経験、ありますよね。そうですよね。皆が通る道です。安心してください。
この記事は、そんなフィードバックを返されていた若かりし僕が、レビューする側の立場になった今考える、コミットの適切な粒度を作るコツと、コミット名の付け方のコツをまとめました。
ここだけの話なので、他言はしないでください。
コミット粒度
コミットの粒度は、どれくらい細かく分割するか？と言った観点だと思って貰えればOKです。
1機能単位で1つのコミットとするのか、極端に言えば1行ごとにコミットをするのか？とかそんな話です。
コミットには、「適切な」粒度があります。
細かくすればするほど良い、というものではありません。
もちろん、バカデカコミットも良くありません。
適切なコミットの粒度にするには、どのようなことを意識すれば良いのでしょうか？
結論から言うと、具体的には、
- 複数のことをやらない
- 書いたコードを説明するだけのコミットメッセージになるようなコミットにしない
- 動く最低限に近づける
といったことを意識すると、ある程度は適切な粒度のコミットになります。
少なくとも、10秒で Request Changes が返ってくることはないと思います。
この3つについて、もう少し詳しく説明します。
複数のことをやらない
1つのコミットでやることは、必ず1つです。
例えば、変更内容を説明するときコミット名に「〜と〜」が含まれるような変更は、大体複数のことを混ぜてコミットしてしまっています。
単純な例を挙げると、「投稿の編集と削除をできるようにする」のようなコミットです。
これは、明らかに「編集」と「削除」に分けてコミットが可能です。
また、コミット名に動作や動詞が２つ以上含まれないようにするのも良いと思います。
例えば、「顧客CSVを取り込んでメールが送られるようにする」のようなコミットです。
これには、「CSVを取り込む」と「メールを送る」という２つの動作が含まれています。
CSVを取り込む機能と、メールを送る機能は分割できます。
ただし、コミット名の抽象度を上げてしまうと、これらの問題は隠蔽されてしまうことも往々にしてあります。
例えば、先のCSVのコミットを「顧客CSV取り込み機能を実装する」のように、「CSVを取り込む処理」と「メールを送る処理」をまとめて一つの「顧客CSV取り込み機能」と捉えてしまう、というようなケースです。
そのため、適切な具体度であることも念頭においておくと良いと思います。
書いたコードを説明するだけのコミット名になるようなコミットにしない
これは、端的に言えば「小さくしすぎるな」ということです。
例えば、「投稿管理用のコントローラーを作成する」というようなコミットは小さすぎる可能性が高いです。
ただし、1文字のタイポの修正とか、条件の追加とか、そもそもコード量が少なく、粒度を大きくすること自体不可能ということもあると思います。
そういったケースはそれはその粒度で大丈夫です。
動く最低限に近づける
コミットはそもそも、そのコミットの時点でアプリケーションが動作している状態であるべきです。
ここでいう「動作」とは、機能として完成しているべきということではなく、落ちない状態であるべき、ということです。
なぜなら、なんらかのインシデントが発生したときに、どの時点の、どの変更で発生したのか？というような特定を行う場合、もし動作しないコミットがあると、再現をしたり原因の特定に余計な時間がかかったりしてしまうためです。
つまり、先の２つの話も含めて考えると、コミットは「小さい方が良いが、動作する粒度」であるべきと言えます。
例えば、コントローラーなどを未作成なのに「新規投稿用のルーティングを定義する」とか、ビューファイルを作ってないのに、「新規投稿画面表示アクションを定義する」のようなコミットは、確かに小さいけれど、そのコミットの時点ではエラーになります。
例えばLaravelで書くと、
「新規投稿用のルーティングを定義する」
// user.php
Route::post('/posts', [PostController::class, 'store'])->name('posts.store');
のようなルーティングだけ作って、PostControllerが存在していない、またはstoreアクションが存在していないと、エラーになります。
「新規投稿画面表示アクションを定義する」
// PostController.php
public function create(): View
{
return view('user.posts.create');
}
のようなアクションの処理だけ書いて、user.posts.createのビューファイルが存在していなければ、エラーになります。
このような状態は避け、最低限「ルーティング、アクション、ビュー」が含まれていて、アクセスすれば画面が表示できるような状態で、コミットするべきだと思います。
コミット名の付け方
コミット名（コミットメッセージ）は、レビュアーにとっては非常に重要な情報になります。
レビュアーはレビュー時に一つずつコミットを見ていくわけですが、コードを見る前に「このコミットは何をしているのか？」を捉えた上でコードを見ます。そのため、コミット名が変更内容と合っていなかったり、「wip」のような何も情報を得られないようなコミット名は、レビュアーの血管を切らせてしまう原因となります。
先のコミットの粒度の話では、「こういうコミット名をつけることになるようなコミットは、大体中身がこうなってるよ」という見方でしたが、ここでは「コミット名そのもの」に焦点を当てていきます。
レビュアーにとって有益なコミット名にするには、
- なぜ？をできる限り記載する
- 「できるようになったこと」を書く
- prefixをつける
と言ったことがポイントになってきます。
それぞれ解説します。
なぜ？をできる限り記載する
これは、基本的には「変更内容はコードを見れば分かる」という考えに基づいています。
例えばユーザー情報に新たにメールアドレスを追加するとします。
この時コミット名が「メールアドレスを追加する」だと、コミット名から得られる情報は、コードの内容と同じなため、レビューにおいては無価値になってしまいます。
レビュアーはコードを見て、メールアドレスが追加できてそうなら Approved を出すかもしれません。
このような場合は、「SMS認証からメールアドレス認証に変更するため、メールアドレスを追加する」とか「XX連携にメールアドレスが必要なため、メールアドレスを追加する」のように、なぜメールアドレスの追加が必要になったのか？を加えると、レビュアーは「その目的なんだったらこうする方が良いのでは？」というようなフィードバックも可能になり、レビューの質を上げることにも繋がるかもしれません。
「できるようになったこと」を書く
とはいえ、なぜ？っていうか仕様なんだもん！というようなことがあると思います。
「顧客一覧機能が必要なので、顧客一覧機能を実装する」みたいなコミット名にするのも微妙です。
そんなときは、「できるようになったこと」を書きましょう。
「顧客一覧を表示できるようにする」といった具合です。
新規機能開発のコミットなどは普通、「できなかったことができるようになった」という変更なので、それをそのままコミット名にしてあげよう、ということです。
prefixをつける
変更には、「新規機能追加」や「バグ修正」、「リファクタリング」などと言った、様々な種類があります。
コミット名でそれが分かるようにしておくのはもちろんですが、もっと視覚的にわかりやすくすると、よりコミット名の価値は上がります。
例えばgitmojiなどを使うと良いです。
- 👍 顧客一覧を表示できるようにする
- 🐛 予約一覧でN+1問題が発生していたので解消する
さいごに
最後まで読んでくださり、ありがとうございます。
いかがだったでしょうか。
コード自体はちゃんとしててもコミットが破茶滅茶だと、レビューには相当な時間がかかります。
コミットの粒度・コミット名が適切になっているだけで、レビュアーの負担はかなり減ります。
それこそタイトルにもあるように、1/3になると言っても過言ではないと思います。
1年目の方や、今までレビュー文化の無かった会社からレビュー文化のある会社に転職した方など、少しでも参考になったら嬉しいです。
特に最初は、レビュアーがどういったことを考えてレビューをしているのか？もわからないことから、レビューしやすいPR、コミットなんてわからない！という人も多いと思います。
レビュアーがどういうことを考えながらレビューしているのか？については、こちらの記事が非常に勉強になるので、一読してみることをおすすめします。
Discussion