みんなが幸せになれるPullRequestの作り方を考えてみた

前提として

Pull Requestの運用方法など、各プロジェクトによって方針が定められているところもあるかと思います。

その上、この記事ではあくまで筆者の考えを殴り書きしているだけのものですので、基本的にはプロジェクトの方針に沿った運用を行い、この記事は参考程度にしていただくのが良いかと思います。

概要

昨今の開発では、GitHubを利用される方が大半だと思われます。
その中でも、ほとんどの人が利用している機能として「Pull Request」が挙げられます。

Pull Requestは「実装者が実装した機能・コードの品質をレビュワーがチェックする工程」であり、「実装者とレビュワーのコミュニケーションを取る場」となります。

Pull Requestも万能ではなく、そもそもレビューも実装も人間が行う（2023年時点だとまだ人間がやってる）ので、確認漏れや認識のずれが発生してしまうこともチラホラあります。

また、Pull Requestはテキストベースでのやり取りとなりますので、思った以上にコメントの意図が伝わらなかったり、曲解して受け取ってしまうこともありがちです。

Pull Requestはテキストベースでのやり取りとなりますので、思った以上にコメントの意図が伝わらなかったり、曲解して受け取ってしまうこともありがちです。

特に筆者はこれになりがちなので、改めて自分の経験を振り返りつつ、「どんなPull Requestがお互いにとって幸せなのだろう」ということを考えてみようというのが今回の記事の趣旨となります。

今回は「Pull RequestがOpenされた」までではなく「Pull RequestがOpenされ、Merge or Closeされた」までの工程を含めて「Pull Requestの作り方」とします。

Pull Requestの工程で求めたいこと

まず、Pull Requestの工程で求めたいことを実装者とレビュワー目線で考えてみます。
（ほぼ筆者の個人的な感覚になりますが）

実装者側

実装物の品質を見てほしい

まずは「実装したものの品質」を見てほしいですよね。

• 実装したものが要件（もしくは着手したタスクの目的）を満たしているか
  • 画面やコンポーネント
    • あらかじめ提示されたデザイン通りか
    • 特定の条件下で表示崩れが起きないか
    • etc...
  • テストコード
    • 網羅に過不足はないか
    • フレーキーなテストとなっていないか
    • etc...
• 変更部分によってデグレが発生していないか
  • 特にリファクタリング周りのタスクでは重視する必要がありそう
• 変更・追加部分のコード品質はどれほどのものか
  • 変数や関数の命名
  • 言語・フレームワーク等の機能を適切に使用しているかどうか
  • etc...

「実装したものの品質」と言えば簡単ではありますが、見てほしい部分は思っている以上に沢山ありそうです。そのため、「具体的にどう言った点を重点的に見てほしいのか」を何かしらにまとめてレビュワーに伝えてあげるのが良さそうです。

また、諸事情により実装ができなかった箇所などあればその旨を記載しておくと、レビュワーが指摘する手間が省けそうな気がしますね。

改善点があれば記載してほしい

品質チェック段階なので、改善すべき点は見つかれば見つかるほど良いです。
デザイン崩れがあればその再現条件やスクリーンショット、コードで変更したい命名などあればその修正差分を提示してもらえると修正がスムーズに行えそうですね。

改善点だけでなく、良かった点も教えてくれると嬉しい

先述した2点ほど重要な点ではないですが、改善点ばかりコメントされると少し気が滅入ってしまいそうです。
もし余力があって、実装の中にGoodな点があればそれもコメントしてもらえるとモチベーションに繋がりそうですね。

指摘の言葉でダメージを受けたくないなぁ

（これはほぼ筆者の願望ですが）
どうしても「指摘されている」という気持ちが前提となってレビューを否定的に受け取りがちになるので、せめてレビューの内容が攻撃的でなければ心持ち的には楽ですね。

レビュワー側

レビューすべき箇所を明示してほしい

実装物の品質を見るにあたり、レビュワーとしては「何を実装したのか、なぜ実装したのか」が明確になっているとレビューがスムーズに行えそうです。

• 趣旨
• 実装した背景
  • 実装チケットのリンクなど貼っておくと良さそう
• レビューが必要な箇所
  • 実装した箇所の明示
  • レビュー方法
    • 実際に開発環境を動かしてほしいのか、どのように動かせば良いのか
  • 重点的にレビューして欲しい箇所

これらはPull Requestのdescriptionやタイトルに記載することになるかと思います。プロジェクトによって記載内容の方向性など変わってくると思うので、新規参入者にも伝わりやすいようPull Requestのテンプレートが欲しいところです。

また、細かな部分で補足事項があるのであれば、コメントにメモとして残してもらえると良さそうです。

コミット粒度・履歴を整えておいてほしい

全体の変更差分を見るだけでもレビューは可能ですが、全体の差分が多い場合には１コミットごとにレビューする箇所を絞ってレビューすることもありそうです。
このため、コミット粒度を整えてもらったり、コミットメッセージについても変更内容が伝わるようなものにしてもらえるとレビューが捗りそうですね。

趣旨のそれた変更はなるべく差分に含めて欲しくない

趣旨のそれた変更（優先度の低いリファクタリングや他タスクですべき実装）が差分に入ってしまうと、レビュワーにとってノイズとなってしまい、レビューの負荷が上昇してしまうので、できるだけこれは無くしてほしいところです。

Pull Requestを作っていく中で意識したいこと

先述した内容を踏まえ、各視点で意識したいことを書き出してみます。

実装者側

Pull Requestのdescriptionとタイトルを過不足無く充実させる

Pull Requestをレビューする際にまず目に入ってくるのが、Pull Requestのdescription・タイトルとなります。
ここを見るだけで、以下の点が大まかに把握できるようにしておくと良いでしょう。

• このPull Requestの趣旨と目的
• 大まかな実装した内容・変更箇所
• レビューして欲しい箇所とレビュー方法

❌（タイトルから趣旨が読み取れない・余程のことがない限りはdescriptionに記載すべき項目が何かしらあるはず）

🔺（内容に不足がある）

⭕️

（ブランチ名適当ですんません）

descriptionについてはあらかじめテンプレートを作成することが可能ですので、プロジェクトによってテンプレートの内容をあらかじめ検討しておくのがベストかと思います。

Gitの機能を使ってコミット粒度・コミットメッセージ等整えておく

単純に全体の差分を見るだけでもレビュー自体は行えますが、ある程度差分を絞って重点的にレビューしたい時などにコミット単位での差分を確認したりすることがあります。
このため、コミットログについても粒度やメッセージを整えておくとレビューのしやすさが向上します。

❌（とりあえずコミットしとこ〜的な感じでメッセージが曖昧だったり、同一のような内容のコミットが複数発生している）

例えば、以下のような工夫ができるかと思います。

git commit --amendで直前のコミットに変更を加える・コミットメッセージを変更する

git commit --amend

上記コマンドで、現在の変更を直前のコミットに加えることが可能です。

git commit --amend m 

同様に、直前のコミットメッセージを修正することもできます。wipコミットをまとめたりした後にコミットメッセージを意図の伝わるものにするような時に使いましょう。

git reset --softを使用して一度コミットを取り消し、整理し直す

git reset --soft <commit_id>

上記コマンドを使用することで、変更を残したまま指定したcommit_idのコミット直後まで戻ることが可能です。

一つ難点として、各ファイルの最新の変更のみが残るので、コード行ごとの詳細な変更記録は残すことができません。

wipコミットなど整理する際に使えると良さそうです。

rebaseを使って整理する（難易度高め）

rebaseは使用する難易度が多少高いですが、使いこなすことで柔軟にコミット履歴を整理することが可能です。

rebaseの使い方については以下記事に詳細にまとめてありましたので、引用させていただきます。

補足事項はコメントとしてあらかじめ残しておく

「あえてこのような実装を行なった」,「ここは別のPRで対応したい」,「仕様が固まっていないので仮の実装」といったことはレビュワーの前提知識には含まれていませんので、レビューの工程で指摘が入る前に事前にコメントしておくと良いかと思います。（もしくは、Pull Requestのdescriptionに追記しておくなど）

example

趣旨のそれた変更は別Pull Requestへ切り出す

変更内容によっては難しいかも知れませんが、趣旨のそれた変更が混じってしまった場合にはその差分のみ別のPull Requestへ切り出しましょう。
趣旨からそれた変更も残したままにしてしまうと、差分の量によってはレビュワーの負担となり得ます。

例えば、以下のような方法で別ブランチに変更を切り出せます。

git checkoutでファイル・フォルダごと切り出す

git checkout <ブランチ名> <ディレクトリパス>

指定したブランチの特定のファイル・ディレクトリをコピーすることが可能です。
特定のディレクトリ下の変更のみ別Pull Requestに切り出したい時や、大雑把にディレクトリをコピーして不要な変更を取り除くなどする時に使うと良さそうです。

git cherrypickで特定のコミットを切り出す

git cherry-pick <commit_id>

git logなどでcommit_idを把握していれば、cherrypickによって特定のコミットの変更をそのまま取り出すことができます。
この方法の場合、すでにコミット履歴が整備されていることが望ましいです。

いずれかの方法で別PRに切り出し、マージできたらrebaseなどでいい感じに差分を整理しましょう。

レビュワー側

攻撃的なコメントを避ける

「なんでこんな実装になってるん？？」

「チケットの内容と全然違うじゃないか...」

といったことがあるかも知れませんが、余程のことでなければ攻撃的な表現は避けてフィードバックを行いましょう。

❌（流石に敬語は欲しいところ...）

チケットの内容と実装が全然違う。どういうこと？

🔺（捉え方によっては攻撃的に見えてしまうかも？）

チケットの内容と実装がずれているんですが、どういうことですか？

⭕️

チケットの内容と実装がずれていそうですが、何か背景などあれば教えていただきたいです！

実装の間違いを指摘するのは大切ですが、Pull Requestは実装者とレビュワーのコミュニケーションも兼ねているという前提でコメントできるのがベストだと思います。
コメント一つでその人との心理的安全性を損なう危険性があることを念頭においておくのが大事かも知れません。実装者は敵ではなく同じ開発を共にするチームメイトです。
（過剰かもですが、僕はそのように考えているので書いておきます🙏）

気をつけないといけない点として、表現によってはレビューで指摘した情報に確証が無いかのような表現にも見えてしまうことがあります。

〇〇かと思います,〇〇が良さそうです

明らかに修正した方が良いような箇所については、確証を持った表現でコメントする方が良いかと思います。

〇〇にしていただきたいです,

改善点があれば変更して欲しい内容と理由を明記する

改善して欲しい点があればその旨をコメントしましょう。
ここで大事なのは、変更して欲しい内容に合わせてその理由を明記しておくことです。

レビュワーも人間なので間違うことがあります。
実は実装が正で、レビューで指摘した内容が間違っていた...
なんてことも時にはあるはずです。

こういった事態を避けるために、指摘する際に

自分がここを指摘しようとした原動力って何だ？

今行おうとしている指摘が正である証拠はあるか？

これを再確認すると良さそうです。

単に実装者側の落ち度・認識漏れであれば、修正すべき理由など伝わるので説得力が増しますし、レビュワーが指摘しようとした内容が間違いであることに気づければ、間違った情報を未然に防げて万々歳です。

ここでの情報収集の時間も、正確なレビューする上で外せない大事な時間です。

❌（修正すべき理由となり得るソースが明示されていない）

この実装はアンチパターンなので修正した方が良さそうです！

⭕️

公式ドキュメントによると、この実装はアンチパターンとして扱われているので修正の必要がありそうです！
参考：（情報のソースを明示）

特定の条件下でのみ起きるような不具合などあれば、再現条件を含めてコメントする

レビュー中に予期せぬ不具合を見つけることもあるかと思います。
仮に実装者がその不具合を見つけているのであれば修正を行なっているはずなので、実装者はその不具合が発生することを知らないという前提で指摘するべきです。

そのため、指摘する際には不具合がどのような現象か、具体的に何をした際に再現したかを明記してあげましょう。

❌（不具合の発生条件がわからない・曖昧）

〇〇コンポーネントが表示崩れする場合があるようなので直してください！

⭕️

〇〇画面にアクセスして、ウィンドウ幅を〇〇〇pxまで縮めた際に〇〇コンポーネントが表示崩れを起こしていそうです！

余力があれば改善点だけでなく良かった点もコメントする

レビューはコードの品質をチェックし、問題がある箇所の指摘を行う工程ですが、かといって良かった点をコメントしてはいけないわけではありません。
むしろ、良かった点も併せてコメントしてあげることで実装者のモチベーション向上に寄与できるでしょう。

個人的には「ああ、この人は粗探しだけするわけじゃなくって、自分の実装の良い部分も見てくれているんだな」という気持ちになれるので、これをしてもらえると割と幸せになれます🙌

example

〇〇のライブラリを使われているの、とても良いですね！

優先度低いけどやれると嬉しいこと

ここまでのことを意識するだけでお互いに嬉しいPull Requestが作成できそうですが、「こういうところも意識できるといいんじゃね？」と思ったことを書き出してみます。
（個人的な感覚とエゴの塊になります。）

コメントの表現をマイルドにする

プロジェクトのメンバーによってこの辺は分かれてくるところですが、絵文字など使って少しコメントをマイルドにしてあげると、実装者とレビュワーがお互い素直にコメントを受け取れるのかなと思います。

絵文字なし

修正していただけますでしょうか。

絵文字あり

修正していただけますでしょうか🙏

絵文字が厳しい！ということでしたら、内容によっては感嘆符を付けるだけでも結構変わるかもです！

感嘆符なし

ご確認よろしくお願いします。

感嘆符あり

ご確認よろしくお願いします！

修正が軽微なものはsuggestion機能を使おう

修正内容が1箇所のみかつ、軽い命名の修正のみのような変更（例えばtypoなど）についてはsuggestion機能を使って修正後の内容をコメントしてあげましょう。

example

おわり

ほとんど筆者のお気持ちみたいになってしまいましたが...みんなが幸せになれるPull Requestの作り方について考えてみました。
レビュワーとしてのコメントについては最近意識し始めているのですが、実装者としてのコミットログの整理などはまだgitの知見が足りていないので、これから経験を積んでいきたいところですね...！

他にもPull Requestの質を上げられるようなものが見つかれば、この記事に追記していこうかと思います！