コードレビューの文化とPRのベストプラクティス

コードレビューは門番をすることではありません。チームで学び合い、担当を引き継ぎ、品質を高く保つためのものです。良いレビュー文化はチーム全体を成長させますが、悪いレビュー文化はボトルネックと不満を生みます。

1) 小さく保ちます（可能なら400行未満にします）。2) 明確な説明を書きます。なぜ変更するのか、何を変更するのか、どうテストするのかを含めます。3) チケットにリンクします。4) UIの変更にはスクリーンショットや動画を追加します。5) レビュアーを依頼する前に、自分の差分をセルフレビューします。

コミットと同じconventionalな接頭辞を使います。feat: add user profile page、fix: handle 404 in fetch wrapper、refactor: extract Avatar componentのように記述します。これらをもとに変更履歴を生成するチームも多くあります。

多くのチームではPRテンプレートを使います。.github/pull_request_template.mdに配置します。

レビュアーを依頼する前に、自分の差分を1行ずつ確認します。わかりにくい判断をした箇所には、その理由を説明するコメントを追加します。多くの場合、他の人に見てもらう前に自分のバグに気づけます。

2000行のPRが丁寧にレビューされることは、ほとんどありません。1) リファクタリング（動作の変更なし）、2) 新しい動作、3) UIの仕上げ、のように分割します。それぞれレビューしやすく、元に戻しやすくなります。

命令ではなく質問として伝えます。「この処理をhookに切り出すのはどう思いますか？」のほうが、「これを切り出してください」よりも効果的です。必須修正と、できれば対応したい改善を区別します。接頭辞としてnit:、question:、blocker:を使います。

「これはわかりにくい」と言われても、作成者は何をすればよいかわかりません。「早期returnの意味を理解するために3回読み返しました。guard clauseとして切り出せないでしょうか？」なら、対応すべき内容が明確になります。

巧みな解決策、わかりやすい命名、役立つテストには、肯定的なコメントをします。そうしたパターンを促せますし、他のフィードバックも受け入れやすくなります。批判だけを受けたPRでは、対立しているように感じてしまいます。

フォーマットはPrettierが処理します。スタイルはESLintが処理します。タブとスペースの違いにレビューの時間を使わないでください。特定のスタイルに関する指摘が何度も出るなら、linterのルールに組み込みます。

テストもコードです。新しいコードにテストがあることを確認します。テストが本当に正しい対象を検証しているかも確認してください。間違った対象を検証しているため、壊れているのに通ってしまうテストは数多くあります。

すべてのコメントに返信します。親指を立てる絵文字だけでもかまいません。納得できない提案には反論します（コードを書いたのは自分なので、自分にしかわからない背景があるかもしれません）。解決したスレッドには解決済みの印を付けます。対象範囲が変わったらPRの説明を更新します。

1営業日以内にレビューします。古いPRでは背景が失われます。作成者は次の作業に進み、ブランチにはrebaseが必要になるからです。1週間放置された大きなPRは、必ず厄介なマージになります。

GitHubのSuggestion機能を使うと、作成者はワンクリックで修正を受け入れられます。文章で「この行をXに変更してください」と伝えるより、はるかに速く対応できます。

コードが正しく、テストが通り、変更内容を理解していて、安全にマージできる場合に承認します。承認することは、結果を共同で引き受けることを意味します。形式的に承認しないでください。読んでいないなら、その旨を伝えます。

自分なら違う書き方をするコードに対して、コードレビューでフィードバックを伝えるとき、どのような姿勢が推奨されますか？

作成者は、スクリーンショットとテストを含む、小さく説明の行き届いたPRを作成します。まずセルフレビューを行います。レビュアーは、建設的で質問を中心としたフィードバックをします。blockerとnitを区別します。良い点を称賛します。スタイルの確認は省き、ツールに任せます。1日以内にレビューします。内容を理解した場合のみ承認します。PRテンプレートでプロセスを標準化します。レビューは門番ではなく、協働のためのものです。