※この記事は元々「Gitのこれやめて！リスト」として2015年11月に投稿したものを改訂したものです。
この記事について
私が個人的にgitとプルリクエストについて、「こうして欲しい」とか「これはやらないで！！」とか思っていることをまとめたものです。
元々は2015年に私がコードレビューをしてる時に気になったことを、あまり推敲もせず思うがままに書いた記事でした。今改めて読み返すと稚拙な文章なのと、他に思うところとがあったりしたので、改めて書き直しました。いいね貰ってるのに書き直すのに若干後ろめたさがあるのですが、よりいい内容にできればと思います。
コミットログがきれいだとレビューしやすい
一人で開発するときはgit使っててもブランチ切らないし、プルリクもださないしで、コミットログも"First Commit"の次が"Second Commit"とかでも支障はないです。しかし、チームで開発するときはコミットログを意識すること（コミットの粒度、メッセージ）はすごく大事だと思います。自分がそれに気づいたのは、チームメンバーの出したプルリクエストを頻繁にレビューするようになってからで、コミットログがちゃんとしてたら変更差分が多くてもレビューしやすく、逆に大量の更新を１コミットとかで作られたりすると、すごくレビューしづらいです。後述しますが、コミットログから「なんでこのファイルに修正がはいるのか？」が読み取れるので、これがないと「なんでこのファイルに修正入るの？」「それは〜」ってやりとりが発生してすごく効率が悪いです。
あとは、いい感じにコミットが切られていると、他のメンバーから「この機能に依存しているから、先にマージしたい」と言われた時なんかに対応できたりして並行開発しやすくなります。また、不幸にもバグが混入してたときに、影響箇所を切り戻すのもやりやすいです。
次に私が考えるいい感じのコミットを記述します。
コミットメッセージにはWhyを書く
色んなところで言われていますが、私もそう思います。具体的にはこんな感じです。
- 契約エンティティに解約の機能を追加
- 会員エンティティを作成
- ユーザーIDで契約を検索した時に複数件ヒットする考慮が抜けていたので追加
- 入会できるかチェックする機能で申込者の年齢もみるようにした
など。逆に個人的に気になるコミットメッセージは以下のようなもの
- XXX.javaを作成（それは見たらわかる。なんでそのファイルが必要なのかを知りたい）
- レビューの指摘を直した（どんな指摘だったのかレビューした人じゃないとわからない）
- 一旦コミット（一旦ってなんだ？？）
ただ、いきなりきれいなコミットを作るのは難しいので、私は、まずは自分が作業しやすいようにコミット切って、プッシュする前にコミットログを綺麗にするってことをよくやります。そのときに主につかうgitのコマンドは以下です。
| コマンド | 用途 |
|---|---|
| git reset soft | resetしたあと、git addと組み合わせてコミットを切り直す。hardをつけるとコードも戻っちゃうのでやってはダメ |
| git rebase -i | コミットをまとめたりとか順番を入れ替えたりとかできる |
コミットもプルリクもとにかく小さく
あいにくコミットとプルリクの粒度についての答えは持っていませんが、私の経験上、経験の浅い人ほどコミットもプルリクも大きくなります。変更差分が大きいとレビュー観点も多くなるので、レビュワーはとても辛いです。
そもそもコードを始める前に設計していない
コミットとプルリクが大きくなる要因はこれだと思います。ちゃんと設計できていれば、「まずはInterfaceを作ってプルリクだして、次に実装クラスを〜」とか、そういった段取りが組めるかと思います。経験の浅い人にチケット（例：申込機能をつくる、など）を渡すと、コーディングしながら設計してるので、作業の切れ目がわからなくて大きなコミットになるのだと思います。
とはいえ、「コード書いてみないとどんな設計がいいのかわからない」って気持ちもわかる気がするので、その場合は一旦捨てるつもりで、スケルトンコードだけでも実装してみたらいいと思います。
モブプロ・ペアプロ
上記に関連して、チームのふりかえりで「コードレビュー出してからの後戻りが大きい」という問題が挙げられたことがありました。Tryとして２〜３人で設計・実装を同じテーブルで随時共有しながら進めるようにしたのですが、これが結構よくて後戻りが減ってパフォーマンスがでました。
その他・gitまわりでよくトラブルになるケース（随時更新）
以下は、gitの操作周りでトラブることが多いパターンを書いていきます。
rebaseでミスる
rebaseはベースになるコミットを変更するから、それ以降のコミットがマージ元とは別になります。この仕組みをしらないと事故ります。例えば
# リリースするからmasterブランチを更新するよ
git checkout master
git merge develop
# Pushする（エラー）
git push origin master
# しまった。master更新するの忘れてた。
git pull --rebase master
とした場合、rebaseしたらその後のコミットはマージ元とは完全に別ものになるのでmergeされたことになっていないです（masterブランチに直接コミットしたことになる）よって、以後developをマージする際に必ずコンフリクトが起きます。