背景

フロントエンド（React×TypeScript）開発のPRレビュー担当（以下レビュアー）になり、約半年がたちました。
正確には数えていない（数えられない）が半年間で大体300~400（diffの大小を問わない）ほどのPRレビューを行ってきて、僕自身、レビュアーの経験が初めてだったのでレビュアーとして難しかったこと、こんなPRはいいなあと感じたことや気をつけていたこと、できなかったこと（色々あるが...） についてまとめようと思います。

レビュアー視点の考えが多めですが、同時にレビューをしてもらう側（レビュイー）でも重要なことだと思います。

自分自身初めての経験ではあったので、改善とはいかなかったものもあり、是非とも他のレビュアーがどのようにレビューを行っているのかコメントでお聞かせいただけると幸いです。

レビュアーになって難しいと感じたこと

①自分の業務と並行して行う場合、時間がなくなる

これは現場、チームによるかもしれませんが、レビュアーだけを業務として行っている人は少ないのではないでしょうか？
自分の担当業務＋レビューを行っているケースが多いと思います。そうなると自分の担当業務を行う時間とレビューを行う時間の配分を考える必要があります。
自分の担当業務に時間を割かなければ納期に間に合わない可能性があり、レビューの時間を削ればPRは溜まっていき場合によっては全体の開発が止まってしまう可能性がある。
この時間の配分が難しかったです。

①の対応策

もはやPRのクオリティとdiffの大きさ、レビューをどれだけ早くできるかの話になってしまう気がしてます。
自分は全体の進捗、自分の業務の進捗、PRの納期（なるべく早くマージしないといけないかどうか）を考慮してケースバイケースで行っていました。

具体的なケース

・全体の進捗が遅れている or なるべく早くマージしたいPRがある場合
これが一番難しいです。早くマージしたいからと言って適当にレビューをするのは逆に後々のリファクタのコストがかかります。
自分的に以下の方法が考えられると思います。
・そのPRを優先してdiffのみを（できればファイルを見た方が直良）全体的にレビューしまとめて修正をお願いする。PRの返却（修正を依頼するのは）は多くても2回以内までになるように1回のレビューでdiffに対して万番なくレビュー＆コメントをする。

・細かいところの修正はマージしてから別途リファクタのチケットとして対応してもらう。（レビュアーが対応する場合もある）。個人的にはこちらはおすすめしません。理由は後述します。

マージを急ぐPRがない場合
この場合は自分の業務を優先し、キリのいいタイミングでdiffが小さいものから順にレビューし、溜まっているPRを減らしていくようにしていました。
またあまりよろしくないですが、軽微な修正はこちらで（レビュアー側）で直してしまうこともありました。その場合後からレビュイーに質問された時は、明確な理由と意図を説明できる必要があります。

②コードは書くより読む方が倍疲れる。＆ストレスが溜まる場合がある。

レビューを日頃行っている方ならご理解いただけると思いますが、レビューにはかなりの体力がいると思います。僕自身、最初の方は毎日クタクタになってました。PRが溜まっていくと心理的な不安も出てきます。
また、人間だから仕方がないと思いつつ、イライラしてしまうことも...
レビュイーへのRespectは欠かさないように。イライラした時は休憩を忘れずに。（僕はこれが本当にできなかった子供です）

③これでも問題ないんだけど「ん〜〜」となってしまう時

現状でも問題がないがこうした方がいいな〜て時がある際は、①と同じだが明確な理由を言語化できる必要がある。またコメントを書く際も 「ここは〇〇なため個人的には△△のような書き方がいいと思いました。◇◇さんが××にした意図などがあれば教えていただきたです。」 などの文言にするといいかなと思いました。
僕は感覚的な理由の言語化がかなり難しいと感じてます。

こんなPRはいいなあと感じたこと（レビュイーがPRを出す際に気を付けるポイント）

①できるだけ小さいdiff（単位）でPRを送るようにしてほしい

たまにではありますが、4ページ、5ページにも及ぶPRがあります。こういったPRを正確にレビューをするのは難しいですし体力をかなり使います。
diffが大きければ大きいほどレビューには時間がかかりますし、レビュー漏れも出てきてしまう可能性もあがります。
チケットによっては仕方ないこともあるのですが、なるべくチケットも小さい粒度で切るのが重要だと思います。

②このPRを作るに至った理由と、どのような対応をしたのかの概要を簡潔に書いてもらえると助かる

これもたまにですが、僕は今なんのPRを見ているのか？となることがあります。なぜこのようなPRが生まれたのか、PRの目的（一つであると直良）、マージする必要があるのか、どのように対応したのかが分からないケースです。レビュアーの視点からするとレビューの際に見るポイントが明確にならないのと、レビュイーに問い合わせるコミュニケーションコストがかかります。

概要（PR名も含む）をしっかり書いてあることによる利点を以下にまとめます。
・重要な機能の改修でレビューを優先するものなのか、そうでないものなのかもここからある程度判断することができる。
・マージ後に行うこと（ドキュメントの作成や、今後の付随機能の開発など）がわかると、それを前提にレビューできる
・マージ後（期間は問わない）に再度確認する場合に明確にわかる
・diffが大きい場合はレビュー者に注意してみてもらいたい（動作確認も含む）ポイントがあるとやりやすい

以上の理由から最低限チームでPRのルールの統一やPRの概要のフォーマットなどを決めておくのもいいかもしれませんね。

レビューで気をつけていること

①diffではなくファイルを見ること

僕はなるべくdiffだけではなくファイルを見るようにしています。理由は以下があります。
diffだけでは全体の流れが掴めない場合がある（掴めていたと勘違いしてしまう）。
もし他の箇所の改修（リファクタも含む）も必要である場合はdiffだけでは把握できない場合があります。その場合は同じPRで対応してもらう時もあれば、コストがかかるものである場合別のチケットとして対応する必要があります。（自分で対応する場合もある）
そういった負債を把握しておくだけでも、今後の保守の助けになると思います。

②できるだけ保留を作らないようにする。or 保留にしたとしても把握はしておく

①と重なる部分ではありますが、diffまたはdiff以外の場所もリファクタまたは改修する必要がある場合は、なるべく保留にしないように心がける必要があります。またはやむおえず保留にする場合（diffが大きくなりすぎてしまうなど）はできるだけ対応する目処（時間、人）を立て、優先度をつける必要があると考えています。
時間がない時、納期が迫っている時、とりあえず動けばいい！時間（納期）に間に合うのが先だ！開発を止めないのが先だ！後でリファクタしよう！という思考に陥りがちです。

ですが、その後でがくることはない少ないです。
市場の流れや業務の流れはとても早く、後でやろうとした、その後での状態の時には他に優先度の高いやらなくていけないことが出てくるからです。

③他のPRとコンフリクトが起きる可能性を把握する

当然のことですが開発者同士のPRでコンフリクトが起きるかどうかの予測は大体予測つくようになります。
予測するのと同時にレビュアーは、マージする順番やコンフリクトが起きた時の解決方法も予測しておく義務があると思います。

④レビューは文句ではなく要求と提案であるべきということを忘れないこと

最後になりますが、これは一番重要だと思います。結局はレビュイーもレビュアーも人です。（最近はAIとか使ってたりするのかな？）
人なのでミスもすれば、考えも違うし、疲れるのは当たり前なのでお互いをRespectし合うようにすることを心がけましょう。自分も普通にミスをするし、自分がレビューしてもらって、怖いなて思う口調やコメントは必ず控えるようにしましょう。レビュアーだからいう権利があるとか、そういうことでは決してないので。
僕は最近まで本当にこれに気づけなかったというか意識できていませんでした

レビュー時にできれば避けたいこと

自分で対応してしまう問題

こちらはマージまでの時間を短縮するために僕がやってしまっていたことですが、おすすめしませんし、止むを得ない場合を除きやるべきではないです。レビュイーにとって自分のコードを勝手に直された場合、いい気分にはならないですし、その後のPRのレベルも上がっていかなくなります。
みなさんはこんな時どうやって捌いているのだろうか...

レビュアーになって学んだこと

プロジェクトの状態を把握しながら業務を行う思考に変わりました。自分の業務をただこなすのではなく、全体の進捗度合いや、負債のあるコード、機能、今後やるべきことなどを把握できるようになります。
また時間の使い方が上手くなり、エッセンシャル思考（今一番何をやらなければいけないか。頭の中で行動の優先順位を立てる）が身につくようになります。
しんどい時もありましたが確実に成長することができました。

補足

最近気づいたことですが、コードレビューについての大事な観点はGoogle エンジニアリングプラクティスドキュメントにかなり詳しく書かれていましたのでご興味のある方はぜひこちらを参考にしてみてはいかがでしょうか。
https://google.github.io/eng-practices/

Discussion