レビューしやすいPull Requestの作り方

Keywords

Contents

  • 1. Pull Requestのタイトルと修正内容を一致させる
  • 2. 概要を記述する
  • 3. コミットメッセージに気をつける
  • 4. コミットメッセージにプレフィクスをつける
  • 5. テスト項目の作成
  • 6. File Changesが多い場合は気を付ける
  • 7. 参考URl

Pull Requestのタイトルと修正内容を一致させる

仮にPull Requestのタイトルが[おすすめ記事のレコメンドアルゴリズム変更]であった場合に、修正内容にテーブルの更新処理があると、タイトルと修正内容が一致していないため、理解しづらいPull Requestとなります。

そのため、Pull Requestのタイトルと修正内容を一致させることが重要です。

概要を記述する

Pull Requestのタイトルだけでは、修正内容を網羅できない場合もあります。その場合、概要を記述することで、レビュアーの負荷を削減できます。言葉で説明することが困難な場合は、イメージ画像を添付することで、理解が促進されます。

また、一般的な説明の技術として、レビュアーがPull Requestの背景をどこまで理解していてどこまでを理解していないかを把握して、理解していないところから丁寧に説明する必要があります。実装した本人は理解していることでもレビュアーは知らないことが多いです。

コミットメッセージに気をつける

コードの変更箇所を読んでいるときに、ある変更のまとまりに複数の変更理由がある場合、非常に読みづらくなってしまいます。

例えば、「退会している場合は、メールを送信しない。」というコミットメッセージで下記のコミットがされている例を見てみてください。

isRetiredのtrueの場合、shouldSendMailメソッドの戻り値がfalseになっている変更はコミットメッセージと一致しているのでわかりやすいですが、nullチェックの処理やフォーマットのリファクタリングというノイズも混ざっています。

[Pull Requestのタイトルと修正内容を一致させる]という箇所で説明したことと同様に、コミットメッセージと修正内容も一致しているとレビューとしては理解しやすくなります。そのため、今回の修正をするのであれば、機能の追加とリファクタリングはわけてコミットすると良いです。(さらに言えば、リファクタリングとフォーマットも分けた方が良いです)

下記の3つのコミットメッセージなら十分読み手としてはわかりやすいでしょう。

退会している場合は、メールを送信しない。
Objects.isNull()を使用する 
コードフォーマット

コミットメッセージにプレフィクスをつける

さらに、コミットメッセージにプレフィクスをつけると、レビュアーの負荷は低減されます。

さきほどのコミットメッセージを下記のようにした場合のことを考えてみましょう。

feature: 「退会している場合は、メールを送信しない。」
refactor: Objects.isNull()を使用する 
format: コードフォーマット

読み手としては、refactorやformatのレビューでは肩の力を抜き、featureのみ注意してレビューすれば良くなるので、負担軽減になります。

【今日からできる】コミットメッセージに 「プレフィックス」 をつけるだけで、開発効率が上がった話

また、後でコミット履歴を修正したいとなった場合、git rebaseをすれば大丈夫です。git rebaseについては、下記の記事がわかりやすいです。

gitのコミットの歴史を改変する(git rebase) 1 / 2

テスト項目の作成

コードを読んでどのような処理かを理解するのは大変な場合があります。 しかし、テスト項目書や単体テストコードがある場合は、そこに、入力と出力が記載されているはずなので、それを読むとコードの理解の手助けとなります。

また、featureのコミットをする場合は、単体テストコードも一緒にコミットをするというようなルールを作っても良いかもしれません。

File Changesが多い場合は気を付ける

これは私だけに言えることかもしれませんが、変更したファイル数が20以上だと、本来そのPull Requestで解決したいこと以上のことをしてる場合が多いです。 そのため、1つの目安として、File Changesが20以上の場合は、Pull Requestのタイトル以上のことをしていないか注意を払ったほうがよいです。(※使用している言語やFWによっても20という数値は変動することと思いますが)

参考URl