年末年始の時期ですが、エンジニアのみなさんはいかがお過ごしでしょうか？
普段は目の前のタスクに追われがちですが、気持ち的にひと息ついたこのタイミングで、日々行っているプルリクエスト(以下PR)について振り返ってみようと思いました。
PRの出し方(≒レビューの受け方)に関しては、数多くの記事や書籍で様々なベストプラクティスが紹介されていると思いますが、たくさん挙げてもどうせ実践できないので今回は私が思う「特に意識してほしい3つのポイント」をご紹介したいと思います。

想定読者

• エンジニアになりたての人
• どんなPRを出せばいいのかよくわかってない人
• 日頃から雑なPRを出している自覚のある人

1. 巨大なPRを作らない

正直なところこれさえ気をつければどうにでもなる気がしている

NG例

~提出されたPR(フィクションです)~

OMG＼(^o^)／

何がよくないか？

• 数千行の変更は常人が読める物量ではない
  • レビュワーの時間的&心理的な負荷が高く、結果全体としてのリリース速度が低下する
• 「自分(実装者)なら読める！」かもしれないが、レビュワーは基本的にそのコードに対して初見であり、実装者か否かではコードリーディングにかかるコストが全く違う
• そもそもこんな巨大なPRについて、説明をしっかり記載できるだろうか？(反語)
• その他
  • Conflictがおこりやすくなってしまう
  • 何か不具合が発生した際などに、切り戻しが大変

こうしよう

• 大きすぎるPRは避ける
  • 理想は200-300行程度
• 機能単位で分割できるものは分割する
  • DB定義とAPI実装でPRを別にする、など
• どうしても実装が大規模になりそうな場合は途中経過を共有する
  • WIP（Work In Progress）のPRを出して早めにフィードバックをもらう
    • 手戻りリスクの低減
    • 早期に認識合わせができる
    • 実装者(レビューイ)とレビュワー双方の安心につながる

2. タイトル・概要欄をできるだけ充実させる

NG例

No description provided..... ／(^o^)＼

何がよくないか？

• 変更の目的や背景が全くわからない
  • コードのみを見て実装意図を推測することになってしまう
    • 対応内容全体の理解に時間がかかる
• レビューの質が低下してしまう
  • 何を重点的に見るべきかわからないので、見落としのリスクが増加してしまう&一方で全く的外れな指摘をしてしまう可能性もある
• レビュー開始の遅れ
  • 「なんかPR出してくれたけど、、、何も説明書いてくれてないから読み始めるのしんどいな。。」となり、内容を理解するまでレビュー着手を躊躇してしまう
    • 「他のPRから先に見ようかな」になる
• 基本的な部分から、確認事項をレビュワー側から都度質問する必要が出てきてしまう
  • レビューコストの増大

こうしよう

• 自分が初めてそのPRを見るつもりでできるだけ明確な説明を心がける (「これを書けばいい」という絶対の正解はない)

  • 変更の目的と、対応内容の記載
    • 「○○という施策のため、××の画面に△△の機能を追加しました！」など
  • テスト方法の記載
  • 影響範囲の明示 (もしあれば)

• 必要に応じてスクリーンショットや動画を添付する

  • 文章での説明より視覚的な情報の方が伝わりやすい
    • 細かいUIの変更を頑張って文章で説明するよりも、Before/Afterの画像を貼ればそれで済んだりする
    • 画面上どのような挙動になるかも動画があれば簡単に伝えることができる

• ルール化する場合はPRテンプレートを用意するのもgood (弊社チームは利用しています )

3. コメントに対する修正箇所を明確にする

レビューコメントへの対応で、以下のような行為は絶対に避けるべきです

• 複数箇所の指摘に対して「fix」だけのコミットをまとめて作成
• 指摘対応とリファクタリングを同じコミットに混ぜる
• レビューコメントに「修正しました!」とだけ返信

NG例

以下複数ファイルにまたがる大量のdiffが続く ／^o^＼

何がよくないか？

• レビュワーが修正内容を追いきれない
  • どの指摘に対してどの修正を行ったのか分からない
• 修正履歴が不明確になる
  • 「fix」程度の内容だと、コミットメッセージから「何かを修正した」といった程度の内容しか読み取れず、後から変更内容を確認する際に非常に手間がかかる
  • 問題が発生した時に原因特定が困難になる恐れがある
• コミュニケーションコストが増加する
  • 修正内容に対して、レビュワーが再度質問せざるを得なくなる
  • 結果としてレビュー完了までの時間が長くなる

こうしよう

• コミット名は意味のあるものにする
  • 例：「Fix: 〇〇に対するバリデーション処理を追加」
• 指摘ごとに個別のコミットを作成する
  • 複数の修正内容を1つのコミットにまとめない
  • 機能追加、リファクタリング、バグ修正は基本的には別コミットにする
  • 1コミットにつき1つの変更になるように心がける
• 修正内容とコミットハッシュの紐付け
  • レビューコメントへの返信時は最低限該当するコミットハッシュを明記する
    • (下記の例ではハッシュではなくコミット名を貼っていますが、こちらの方がよりレビュワーに優しいですね)

最後に

3つと言いつつ細かい内容含めたら全然3つに収まってないやんけ！っていうツッコミも聞こえてきそうですが、今更直せないのでこのままとさせていただきます。
こんな記事を投稿したらもう雑なPRを出すことができないよ

PR

へいしゃではエンジニア全ポジションで積極採用中です！！