Pull Requestで気を付けていること
個人の心がけ
とにかく開発をしてPull Requestを作る事だけに意識が向いてしまうこともあるかもしれない。
しかし、Pull Requestがたまり続けるのはプロジェクト管理者にも開発者（人やポジションによりそう）にも気分がよくない。mergeタイミングが遅れるほどconflictも起こりやすくなる。
開発タスクはPull Request作成、レビュー、merge、そしてdeploy後の動作確認までが1セットで完了するものであり、完了していないタスクを抱えた状態で次のタスクを着手するというのは何か優先順位を間違っている、もしくは順番の入れ替えで全体の進捗が効率化されることがあると思う。
（最近はGitHub Copilotがレビューしてくれる機能もあり、そのうち全てを人間が見なくてもよくなるのだろうか。）
気遣い編
Pull Requestの説明をちゃんと書く
言わずもがなではある。
（必ずしもコードの変更量と重要度が比例するわけでは無いが）数行の変更であるなどコード差分がすぐ見れるものならそこまで丁寧に書かなくていいと思うこともある。
差分の行に直接コメントを入れる
Pull Request作成後にFiles changedのタブの差分に対して、コメントをつけることができる。
Pull Request本体の説明欄は要旨を書き、詳細を補足したい箇所には該当コードの箇所にコメントする。
レビュー対応後のやりとり
「修正しました」の返信をスレッドでされてもその差分をFileChangesのタブから探しに行かないといけない。これだとレビューする側は手間なので差分が分かるように伝える。GitHubだとcommitのhash値をはると対応するcommitリンクにしてくれる。
このとき、commitの粒度を適切にするとレビュアーも見やすい（複数のスレッドでやり取りしている修正を混ぜ込むとややこしくなる）。
diffが大きくなる時
そもそも大きすぎるPull Requestを作らない方がハッピーであるという前提はあるが、シンプルに要件が大きいときややむを得ない事情で大きくなることはある。
みんなで頑張るしかない。
見切れずに放置されるという事が起こりやすいが、これでは開発が健全に進まない。全文のレビューが現実的ではないと思われる場合は、merge権限がある人（or チーム）を説得できる材料を揃えてmergeを促進させたい。
たとえばテストが充実している、もしくは後に結合テストタスクがあるので細かい問題の洗い出しは後ほど出来る期待があるのでとりあえずmergeして進めたいと説得すること。また、重要な部分だけレビューしてもらって後は信用してもらう、などなど。一概にどうということは無く、プロダクトの性質や開発段階によって事情は異なると思うので、レビューの工数と品質確保のバランスを考え進める。
リファクタリングPull Requestを分割する
開発時にはリファクタリングをしたくなることがある。その結果多くのファイルにまたがる変更があった場合、レビュアーにはノイズが大きくて辛くなる。
これを軽減するため、リファクタリングの大きな差分は先にリファクタリングだけの対応としてPull Requestを作り、機能改修はリファクタリング後のブランチから切ってリファクタリングブランチへのPull Requestを作る。
それぞれのPull Requestでレビューの観点が絞れて見やすくなる。第一弾のリファクタリング方針が採用されるであろう前提が入っているので、この前提が崩れると手戻りは発生してしまう。
リファクタリング方針の合意
急ぎの機能開発と大型のリファクタリングが混ざったPull Requestが誕生すると不幸になる。
成熟したチーム・プロダクトでコード規約・設計が固まっている場合は問題にはなりにくいと思うが、開発の比較的初期段階や他社（他チーム）が作ったアプリを引き継いだときに、バックグラウンドが異なる人たちのかたまりで開発をすると、どう直したいかという感想が個人個人でぶれることがある。
この状態をまず認識したうえで、影響範囲が大きくなりそうなリファクタリングはチームで事前に認識を合わせて作業を進めたい。
こういう方針で作りたい、という意見を通すのはある意味政治のような側面があるように思う。
コミュニケーション面
アナウンス
例えばレビュー依頼時に、すぐ読み終えることが出来るものはその旨を書きながら伝えると早くレビューをもらいやすい。
「数行の変更です。さっとmergeしたいです。」、など。
Pull Requestリンクとレビューお願いしますだけのテキストよりは、要旨や意識したことなど何か一言あると読みに行くモチベーションになりやすいと思う。
アナウンス後
Slack（等）で複数人にメンション付けたスレッドは全員に通知が行くので、個別の話題をここでやらない。
例えば複数のPull Requestのレビュー依頼を投げたときに個別のPull Requestの話をすることは避けたほうがいい（全員に通知が必要なら問題ない、基本的にはPull Request内か別スレッドで必要な人にメンションをつけてやりとりする）。
話題が分岐し始めたら関係ない人にはそこそこ迷惑になる（通知設定にもよるが）。
口頭説明
レビュアーをつかまえて時間をとってレビューをその場でしてもらうと早く済むことは多い。
早くmergeしたいものや込み入った事情がある場合はMTGで5分、10分時間を取ってApproveを取りに行く。
Pull Request作成前に、リファクタリングやタスクの要件は10%しか満たさないが設計が決まる段階のコードで方針の合意を取りに行くというのはしばしば有効に思う。
仕組み編
templateを用意する
要旨、やったこと、動作確認、見てほしい事、くらいの項目があればよいと思う。
.github/pull_request_template.md
静的チェックで済むものはレビューで指摘しないでいいようにする
要するにはCIが整っていればよい。
formatやインデントのずれ、type-checkエラーをPull Requestで指摘するのは不毛。
例えば各種フォーマッターやcspellを導入し、lint-stagedなどと合わせてpre-commitに設定したい。
使ったことが無いがNode.js以外の言語でも使えるpre-commit連動ツールにlefthookもある。
Discussion