はじめに

PRレビューは皆さん好きですかね。私は正直苦手なタスクです。
得意な人がいても好きな人はあまりいないイメージではあります。

私はかなり苦手意識を持っているPRレビューですが、今回はなぜ苦手意識を持ってしまっているのか、それによってどのような失敗があったのかと、これからどのようにして苦手意識を払拭していくかについて、メモぐらいの気持ちで記載できたらなと思います。

まぁここら辺もAIが完全に担ってくれる未来は近そうですね。

なぜ苦手意識があるのか

自分の中で日に日に苦手意識が積もっている中で、調べた際に多く出てきたのが「PRの単位が大きすぎるのではないか、もっと小さい単位で切ってもらいレビュー着手までのハードルをましょう」というものでした。

確かにそれは一理あります。
ファイル変更数が20とかなっていると躊躇はしてしまいます。
それが変更ファイル5とかだと、着手までのハードルは間違いなく低くなりますし、PRを返すまでのリードタイムも短くなるでしょう。

しかし個人的にはこれだけでは根本的な解決ができるとは感じませんでした。

大きいところで苦手意識を上げると理由は2つあります。

1つは「時間」です。
PRレビューは時間がかかるが、マージまで持っていかないと開発ブロックになることも多々あります。
そのため、ほとんど強制的に最優先のタスクとして割り込んできます。

2つ目は「責任」です。
PRレビューはそのPRはマージしても問題ないものかその妥当性を判断するために実施するかと思います。
そのためマージ後に発覚した不具合などは全てレビューアの責任になると感じてました。
そうすると、必然的に1つのレビューに力を入れてしまいます。

PRレビューでの失敗

先ほど挙げた苦手意識の中の「時間」と「責任」によってどのような失敗が起きたか。

基本的に「責任」を果たすために「時間」が犠牲になるかと思います。
ここで犠牲にできる時間があれば、失敗まではいきませんが日々タスクというのはさまざまなものが湧いてきます。

かく言う自分も複数タスクを抱えている状況で、PRを1日に何個も見ることはできませんでした。

それによって責任を果たすだけの時間が確保できず、PR〜マージまでのリードタイムの長期化が起こり作業ブロックによる開発スピードの低下や自身の工数圧迫が発生し、チーム全体の開発体験の低下につながりました。

やらないことを決める

まずはPRを確認する中で、自分が行なっていたフローです。

  1. PRテンプレートの記載項目の漏れがないか確認する
  2.  仕様書の確認
  3.  PR作成されたブランチへ移動し、ローカルにてバリデーションなどを含めて動作確認
  4. ユニットテストの結果を確認
  5.  開発時はテスト全体実行ではないため、カバーできていない箇所は今回の変更差分と影響がない箇所であるかの確認（ここはシンプルに並列実行可能として設計するべきだった）
  6. ローカルでのユニットテストの実施
  7. 変更差分から意図が読み取れないコードがある場合に、細やかに処理を追っていく
  8. 納得できなければ実装者に確認
  9. マージ

かなり無駄が多いなと感じると同時に責任を負いすぎているなと感じます。

ここで感じたのがレビューアが「やらないこと」を定義して周知する必要があるということです。
逆に言うと実装者にやってもらうことの認識を合わせるという初歩的なことになります。

PRテンプレートをガチガチにしてそれに倣っていただくとしても良いですが、やはりテンプレート文だけだと意図が伝わりきらなかったりします。
そのため、やはり実際に開発を行う際にチームで認識を合わせておく必要はあるかと思います。

上記のレビューフローで言うと3~5はレビューアで実施はせず実装者側に責任を持たせるべきだと考えました。

そもそも動作確認において期待通りに動いていないものは実装タスクとして完了できていないものなので、レビューされるPRとして条件を満たせていないように思います。

そのため、動作確認は全て実装者側で実施しエビデンスのキャプチャをPRに全て添付し、その結果だけで動作は確認できるレベルにしていただくようにすることです。
足りないユースケースや考慮漏れは、キャプチャベースでレビューする形です。

そしてレビューアは動作する前提で品質担保におけるソースコードの妥当性をチェックする作業に比重を傾けます。
また、意図が読み取れない場合も実装者にすぐ確認するべきで、細かに処理を追うなんてことはあまりにも効率が悪くあります。

そして考えた結果、下記ぐらいの作業量であるべきではないのかと思いました。

  1.  PRテンプレートの記載項目の漏れがないか確認する
  2. 仕様の確認
  3. 変更差分から意図が読み取れないコードがある場合は実装者に確認
  4. マージ

こう考えるとPRレビューという作業への苦手意識もかなり薄れました。

そして「やらない」と決めたことは、ぶらさないことが自分の性格上かなり大事でした。
「やっぱり不安だから、ローカルに落として確認しよう」などとしてしまうと、ずるずると都度やってしまいます。
その場合は「やらない」と決めたことをぶらさないようにする（不安にならない）環境整備をする方に注力した方が良い気がします。

さいごに

実際ここに記載したやらないことは、前からプロジェクトで実施していたりします。
ただ、自身の性格上、不安になり結局同じことをやってしまったりするので戒める意味合いも込めての記載です。

レビューアとしてそんなに大層なことができる人間ではないので、良い運用があれば教えていただけると喜びます。