コードレビューするコストを少しでも下げるために、自分が実装をするうえでCommitやPullRequestを作る時に気を付けていることの共有です。
対象読者
Gitを使ってコードを実装する人、コードレビューをする人。
特に若手・ジュニアのエンジニアの人。
中堅エンジニアの方には当たり前の話かもしれませんし、別の意見があるかもしれません。
その辺はコメントいただけたら嬉しいです。
基本のおさらい
目次を見て内容が想像つく人は「基本のおさらい」は読み飛ばしてもらって「PullRequestのレビュー対象に順番を持たせる」から読んでもらえばいいかなと思います。そっちが本筋で言いたいことなので。
PullRequestの粒度を小さくする
コードレビューをするうえで、PullRequestの変更量が多いとレビュアーは頭のメモリに置いておく情報が増えてレビュー作業を難しくします。
だいたい2桁のファイルに変更があったり、commitが2桁あると私の感覚的に身構えます。実装者の人もその辺が警戒ポイントです。
私の感覚的には、↓くらいの感覚でいます。
- 実装前に関連する既存コードを読んでコメント追加やリファクタリングをしたらまずそれで1度PullRequestを作る
- 自分が実装する機能のうち、共通で利用される処理を作成・変更したらまずそれで1度PullRequestを作る
共通機能は早めにmergeしないと他の人の作業をブロックしかねないので気を使います。
PullRequestに異なる文脈の作業を入れない
具体的には↓のようなことをしないよう気を付けます。
- 既存箇所のリファクタリングと機能の変更を一緒にしない
- PullRequest対象の変更と関係ないけど、実装中にみつけてついでに修正したようなものを一緒にしない
あとからPullRequestのマージを取り消したい場合に関係ないものまでまとめて消えます。
あるいは関係ない部分が万が一悪さをした場合、本当に入れたかった変更ごとリリースから取りやめになることもあります。勿体ないです。
ファイル名のリネームとリネームしたファイル内の変更を一緒のPullRequestにしない
細かいけど、gitのサービスがファイルリネームじゃなくてファイル削除＆新規作成と勘違いしがちだからやらない方がいいです。
コミット分けても勘違いされがちだからPullRequestごと分けます。
コミットコメントには何がどうなるのかを書く
コミットコメントには何がどうなるのかを書きましょう。
具体例を挙げると↓のような内容でコメントを書くように気を付けます。
- 重複コードの削減と処理の分割を行った
- ユーザー登録に関するバリデーションの単体テストを実装
これだけだとわかりにくいので、避けたいコメントも記載します。
具体的には↓のようなことをしないよう気を付けます。私自身もやりがちなので気を付けてます。
- ○○ファイル修正
- 何が”正”の状態かわからないです
- ○○ファイル変更
- どう変更したのか知りたいです
- レビュー指摘対応１
- 何をどう変えたのかコミットコメントから読み取れないです
変更の起因がわかりにくいような場合は、何故変更したのかも書くとよりレビュアーに親切です。
ここではコミットコメントとして書きましたが、PullRequestのタイトルも同様です。
PullRequestのレビュー対象に順番を持たせる
こちらがこの投稿の本チャンです。
一言に実装するといっても、実装の中にはいくつかの工程があります。
たとえば一部の機能を変更する場合だと私は以下のような工程に分けて作業を進めています。
- これから変更する箇所に関連する処理を読む
- 自分の理解をふまえて既存の処理のリファクタリングやコメントの追加する
- 変更に応じたテストコードを書く
- とりあえずテストが通るように処理を書く
- とりあえず書いた処理をリファクタリングする
上記の工程でもまだ抽象化していて、実際には○○のレイヤーのテストを書く～であったり、○○のバリデーションを作る、などもっと細かな工程に分けて作業を進めています。実装者はこの工程を順番に進めているので、頭のメモリに置いておく情報量は少な目で済みます（前工程の作業が終わったので次の作業にフォーカスする）。
しかし、レビュアーにこの工程の順序をふまえず変更の総量をぶつけると、レビュアー側で改めてレビュー対象物を工程割りして読む必要が出てきます。
この工程割りをする作業が地味に面倒なのです。どのファイルをひとまとまりで読めばいいのか、この工程のレビューは終わったんだっけ？など色々なことが頭のメモリを奪っていきます。
これを回避するために、CommitやPullRequestの説明文を活用してレビュアーが実装者が踏んだ工程の順番を追体験させます。
レビュアーに工程を追体験させるための方法
工程ごとにCommitする
上で書いた工程ごとにCommitをすることで、レビュアーに何の工程をレビューするのかという前提を置いてレビュー対象を絞って見られる状態を作りましょう。
上で書いた抽象化した工程でCommitをするか、変更の規模が大きい場合はもっと詳細な工程単位でCommitを積んでレビュー対象を絞れる状態を作ることを意識することをお勧めします。
PullRequestの説明文に読む順番を書く
CommitのHistoryを古い順に読めばわかる状態を作ることができていれば、PullRequestの説明文に読む順番をあれこれ書く必要はありません。
Historyの古い順に読み進めてください。で終わります。
綺麗にCommitを順番に積めなかった場合でも、説明文で読む順番を書くことでカバーできます。
どのCommitがどの工程の作業にあたるのか、どの順でよんだらいいのかを説明文でレビュアーに伝えましょう。
レビュアーが追体験する必要がない工程は省略する
工程をレビュアーに追体験させるうえで、不要な工程もあります。たとえば上で挙げた工程の例でいくと、「とりあえずテストが通るように処理を書く」をわざわざ追体験させる必要性はないはずです。一度とりあえず書いた荒いコードを読ませなくても、リファクタリングして自分なりに読みやすい状態になったものを読ませれば十分です。この場合、4で作ったCommitはgit reset --mixed して5の状態だけでCommitを作ります。
工程順を破って作業した場合はCommitを作り直す
「PullRequestのレビュー対象に順番を持たせる」の項目で書いたように、実装を進めるうえでいくつかの工程にわけて順番に進めるうえで、過去に終えた工程の作業に少し手を付ける場合があります（実装する中でテストの不足に気づいた等）
その場合はresetなどを使って過去のCommitを消してでも作り直します。
私はたいていreset --mixedでCommitを積みなおしますが、rebaseしてsquashしてもいいですし、commit --amendでうまいこと修正してもしっくりくる方法でCommitを作り直せばいいと思います。
あとから”一部修正”みたいなコミットを積み始めると、レビュアーは工程ごとのレビューをするうえで、「この実装がおかしいけど後から修正したってコミットが添えられていたからそこで直ってるかも？」という情報を頭に置きながらレビューすることになります。
本当に規模の小さい変更であればPullRequestの説明文に「○○の工程の作業だけど、後からこのコミットも足したので合わせて読んでください」的なコメントを残して許してもらう場合は私もありますが、基本的に工程を破って作業した場合はコミットの作り直しをおすすめします。
作業の進捗をリモートに細かく残したい場合
DraftのPullRequestを作って会話したい、作業の進捗感を共有したい、など色々な理由でレビューする段階まで終わっていない変更をリモートサーバにpushすることがあります。
この場合、後からCommitを作りなおすことが難しくなります(rebaseをつかっていれば綺麗に作り直せるかもしれませんが)。
そのような場合、私は一度リモート側のブランチを削除します。手元でCommitを作りなおしてから改めて再度pushして解決しています。
（地味に手間なので敬遠していましたが、優秀なエンジニアの人がレビューの前にコミットまっさらにして作りなおすという話を聞いて反省してからは割と私もやっています）
生成AIでいっきに全工程の成果物を作らせた場合
Agentモードの生成AIに作業させた場合、上記の工程の作業を纏めていっきに生成するケースがあります。その場合、レビュアーに工程を追体験させるための方法は、泥臭く自分で変更内容を工程ごとに分類してCommit作成するに尽きています。
（AIに工程ごとのコミットを作成させることがベストだと思いつつ、うまくコントロールできていない...いい解決策でもプロンプトでも持っている人がいたらぜひコメントください。)
私は結局、レビュー作業が大変なので、Agentモードでコードをまとめて書かせるという使い方より、自分が実装中の工程の範囲内で小まめにAIに手伝わせるような使い方が多くなっています。
最後に・感想
ここで書いたことを実践すると、実装者の作業時間は間違いなく増えます。
しかし、生成AIにコーディングをサポートさせることで、コードの生産量が増える一方で、コードレビューの部分がボトルネックになりがちかなと感じています。
ボトルネックがコードレビュー部分にあるのであれば、少々実装の作業量が増えようが、レビューコストの削減に取り組むほうが、最終的な成果物量は増えるはずです。
(ボトルネックが実装部分にあるのであれば、ここで書いたことを意識せずにとにかくPullRequest作るほうがいい場合もあります。そこは気を付けてチームを観察してください。)
また、レビュアーを楽にするという書き方をしましたが、自己レビューを楽にすることにもつながることを意識してください。
自分で自分が書いたものをレビューするためにもPullRequestやコミットをレビューしやすく保ちたいですね。