はじめに
Pull Request するときのルールって会社やプロジェクトごとに定められていることが多いと思うのですが、細かく覚えているかというと、そんなこともない。というよりも、Pull Request するときにルールが記載されているドキュメントを軽く見返したりしているうちに自然と覚えてしまうことが多いです。
そのため、あらためてに人に説明するときに何が大切だったかなと困ってしまう場面がありました。Pull Request するときに推奨されているようなルールって何だろうなと思いつつ、何を気にしているかを書くことにしました。
変更内容は小さいほうがいい
１つの Pull Request では１つだけの変更内容が望ましいです。変更内容が多いとレビューコストが高くなったり、何のための変更なのかが分かりにくかったり、マージしたときの変更影響が大きかったりするためです。ただし、小さければ良いということもなく、変更内容のコンテキストによって判断すべきかなと思っています。どうしても変更内容が多くなってしまう場合は、その理由を Pull Request 内に記載しておいてもらえるとレビューしやすくなります。
変更内容はひとつにする
これは単一責任の原則に基づいているのですが、Pull Request も同様であるべきかなと思っています。変更内容が複数あると、後で見返した時に変更内容が分かりにくかったり、マージしてリリースして問題があった際に、原因分析で時間がかかってしまう要因になります。とはいえ、新機能を追加するときなどは変更内容が複数ある場合があり、その場合は同期的なコミュニュケーションをとって、議論した上で Pull Request をするとハイコンテクストな状態になりベストかなと思っています。
テストが書かれていること
テストのないコードはレガシーコードと定義されているくらい、テストは大切です。テストがないと開発者の意図したとおりに変更されているかが分かりにくいので、テストコードがあって検証内容が自動化されていることが望ましいです。まれに既存のコードを修正するときにテストコードがない場面に遭遇すると思いますが、修正するタイミングで修正しないと修正する機会は中々訪れないので修正できるときに合わせてテストコードも追加したいところです。
コンテキストを書く
コードを書いている人は、何のために変更する必要があって、どうしてこのようなコードになったのかが分かりますが、レビュアーは一緒に活動していない限り分からないことがあります。Pull Request に記載してほしい内容としては 5W1H の How も書くよりも When, Where, Who, What, Why を多く書くようにしてもらえるとレビューしやすくなるかなと思います。
おわりに
Pull Request するまでに開発者は、多くの労力を費やしていることが多くて早くリリース（開放）したい想いが強くなりがちかなと思っています。しかし、レビューをしてみると分かるのですが、背景や目的、理由が記載されていない Pull Request を見ると承認すべきかの判断が遅れてしまいます。急がば回れの精神で丁寧に Pull Request を作成することが大切ですね。
あらためて書いてて思ったのですが、いきなり聞かれても即答できないなと感じています。定期的に聞かれないと説明できなくなってしまうので、聞かれるような機会を作った方がいいかもしれません。
