コードレビューの書き方

コードレビューは、新人が「このチームでは何が正しいとされるのか」を学ぶ、 最も回数の多い接点です。1週間に何度も届き、しかも文章として残ります。

そして同時に、最も簡単に人を萎縮させられる場所でもあります。 悪意なく書いた一言が、相手には全否定として届きます。 この章は、その事故を減らすための具体的な書き方の話です。

まず速さ。遅いレビューはどんな良い指摘より害が大きい

書き方の前に、速さです。順番を逆にしないでください。 丁寧で的確なレビューが4日後に届くより、雑でも当日返る方が、新人にとっては価値があります。

レビューが遅いと、次のことが同時に起きます。

• 新人は次に進めない。依存する作業がある場合、完全に手が止まります
• 本人が自分のコードを忘れる。3日経つと、なぜそう書いたかの記憶が薄れ、指摘の理解に余分な時間がかかります
• PR が積み上がる。返ってこないので次の作業に着手し、レビュー待ちが2本3本と増えます
• コンフリクトが増える。その間に main は進みます
• 「催促していいのか」で悩む時間が生まれる。新人はこれを言い出しにくく、黙って待ちます

4日待たされた新人は、5本目の PR を出す頃には「どうせすぐ見てもらえない」を前提に動き始めます。 これは態度の問題ではなく、環境への合理的な適応です。

目標時間を決めて、チームで共有する

「気づいたら見る」は運用ではありません。数字を決めてください。

重要なのは一次反応です。全部読む時間がなくても、読めない旨を伝えるだけで待ち状態は解消します。 新人が本当に困るのは、反応がないことによって「見られているのかどうかも分からない」状態です。

指摘の書き方

ここからは文面の話です。実際に打ち込む文字列で示します。

なぜダメかを書く

最も多い失敗は、結論だけ書いて理由を書かないことです。 書いた側は自明だと思っていますが、新人にとっては「そういうルールがあるらしい」で終わります。 理由がないと、次に似た場面が来ても応用できません。

悪い例:

ここは Map を使ってください。

良い例:

must: ここは配列の線形探索なので、items が増えると
リクエストごとに 件数 × ループ回数 の比較が走ります。
1000件を1000回引くと100万回です。
 
先に Map を作っておくと引くのは O(1) になります。
 
  const itemById = new Map(items.map((item) => [item.id, item]));
  const found = itemById.get(targetId);
 
件数が常に数十件で済むと分かっているなら、今のままでも実害はありません。
そのあたりの見込みが分かれば教えてください。

長いと感じるかもしれません。しかし、この1件で相手が学ぶのは 「Map を使え」ではなく「データ量から計算量を見積もる」という考え方です。 同じ指摘を10回書かずに済みます。

人ではなくコードを主語にする

主語が人になると、技術的な指摘が人格の評価に変わります。書いた側にその意図はなくても、 受け取る側は「自分が駄目だと言われた」と読みます。

悪い例:

このあたり、非同期処理をあまり理解できていない感じがしますね。

〜さん、いつも例外処理を書き忘れますね。気をつけてください。

良い例:

must: この関数は await が抜けているので、DB 書き込みの完了を待たずに
レスポンスを返します。失敗しても呼び出し元は成功として扱います。
 
- if (tx.commit()) {
+ if (await tx.commit()) {

must: 例外が発生した経路でコネクションが閉じられません。
負荷時にコネクションが枯渇します。defer で閉じるのが安全です。

判定は単純です。その一文の主語が「この関数」「この処理」「この分岐」なら安全で、 「あなた」「〜さん」「この書き方をする人」なら危険です。

質問形で圧をかけない

「なんでこうしたんですか？」は、書いた側の主観では純粋な質問です。 しかし新人には詰問として届きます。とくに文章では表情も声色もないので、防ぎようがありません。

悪い例:

なんでここ再帰にしたんですか？

これ、動作確認しました？

良い例:

q: 再帰にしたのは、カテゴリの階層が不定の深さになる想定でしょうか。
もし最大2階層と決まっているなら、ループの方が読みやすいかもしれません。
前提を教えてもらえると判断できます。

q: 手元でどこまで確認したか教えてください。
異常系（在庫0のケース）が未確認なら、こちらで一緒に見ます。

型は共通しています。まず自分の推測を書き、その上で確認するという順番です。 推測を先に出せば、相手は「合っています」か「違って、実は〜」と答えるだけで済みます。 「なんで」から始まる文は、相手に弁明を要求します。

良い箇所も書く

指摘だけが並んだ PR は、量が多いほど全否定に見えます。 新人はまだ「レビューは良し悪しの評価ではなく作業の一部」という感覚を持っていません。 1件でいいので、良かった点を具体的に書いてください。

praise: エラーログに request_id を入れてくれたの助かります。
障害調査で他のログとそのまま突き合わせられます。

praise: このテスト、境界値（0件・1件・上限）が揃っていて良いです。
このパターン、他のテストでも真似してください。

「よくできました」のような内容のない褒め方は逆効果です。どこが、なぜ良かったのかを具体的に書くと、 それ自体が「このチームで評価される基準」を伝える教材になります。

褒める対象は、成果物より判断にすると再現性が上がります。 「早めに相談してくれたので手戻りが小さくて済みました」 「仕様が曖昧な部分を実装前に確認したのは正しい判断です」のように、 次も同じ行動を取ってほしいものを名指ししてください。

指摘の強さをラベルで示す

ラベルがないと、新人はすべての指摘を「必須の駄目出し」として受け取ります。 「ここ、こう書いた方が好みかも」という軽い一言も、全力で直そうとします。 結果、些細な指摘に時間を溶かし、しかも精神的には10件の否定を受けたことになります。

行頭に短いラベルを付けるだけで、これは大きく改善します。

nit は "nitpick"（あら探し）の略で、 「これはあら探しだと自分でも分かっている」という宣言です。 だから nit: を付けたコメントで承認を止めてはいけません。付ける意味がなくなります。

導入の仕方も簡単です。チームの規約に上の表を貼り、新人には最初の PR を渡す時に 「nit: が付いているものは直さなくていい」と口頭で1回伝えるだけで機能し始めます。

承認の基準を決める

レビューが長引く原因の多くは、技術ではなく基準が共有されていないことです。 とくに問題になるのが「自分ならこう書く」です。

「自分ならこう書く」は、承認を止める理由になりません。 コードには常に複数の正解があり、レビュアーの書き方はその一つでしかありません。 これを止める理由にすると、レビュアーが変わるたびに要求が変わり、 新人は「誰に当たるかで結論が変わる」ことだけを学びます。

承認を止めてよいのは、次の4つです。

逆に、次は止める理由になりません。

• 変数名の好み（明らかに誤解を招く名前は別。それは「壊れる」に近い）
• ループか高階関数か、早期 return かネストか
• コメントの多寡
• 自分の書き方との違いそのもの
• テストの書き方の流儀（テストが無いのは別問題）

チームでやることは1つです。上の「止めてよい4つ」を文書にして合意を取ること。 レビュアー間で基準が揃っていないと、新人は基準ではなくレビュアーの機嫌を学習します。

指摘は絞る。10件の細かい指摘より、根本の1件

新人の PR には、指摘したいことがいくらでも見つかります。全部書きたくなりますが、 20件のコメントが付いた PR を受け取った人は、内容を理解する前に消耗します。

そして本当に問題なのは、20件の中で最も重要な1件が埋もれることです。 細かい指摘と重大な設計の指摘が同じ見た目で並ぶと、相手は上から順に潰していきます。

絞り方は次のとおりです。

1. まず must: に該当するものだけを拾う
2. その中で、他の指摘の原因になっているものを1つ選ぶ。例えば「この責務がこのクラスにあること」が直れば、派生する5件は消えます
3. 残りは nit: にするか、書かない
4. 同じ種類の指摘が複数箇所にあるなら、1箇所だけ書いて「同じものが他に3箇所あります」と添える

4 は効果が大きい割に軽視されています。同じ内容を5箇所に貼ると、 相手には5つの失点に見えますが、実際には1つの学習事項です。

往復が3回を超えたら、話す

文章のレビューには限界があります。次のような時は、テキストを続けるほど遅くなります。

• 同じ論点で3往復した
• 相手の返信から、こちらの意図が伝わっていないと分かる
• 設計の選択肢が複数あり、トレードオフの説明が必要
• 相手が明らかに困っている、あるいは長文で弁明を書き始めた

15分の通話や画面共有で終わる話が、テキストだと2日かかることは普通にあります。 「ちょっと通話しませんか。5分で済むと思います」と誘ってください。

PR が大きすぎる時

1000行の PR が来たとします。分割を依頼するのは正しい判断です。 大きい PR はレビューが雑になり、雑なレビューは事故を通します。

ただし、「大きいので分割してください」だけで終わらせないでください。 新人は「どこで切るのか」を知らないから大きくなっています。 切り方を教えないと、次も同じサイズで出てきます。

具体的な切り方の例:

そして、今回の分の扱いを決めます。すでに書き終わっているものを分割させるのは、 新人にとってかなりの負担です。

• 今回はそのまま見る。ただし次回から切る場所を事前に相談する
• 今回はコミット単位で順に見る（git log の粒度が揃っているなら有効）
• 今回も分割してもらう。ただし切り方を一緒に決める

どれを選んでも構いませんが、「次回から小さくするための約束」までを必ずセットにしてください。

そもそも 1000 行になる前に検知するのが最善です。 着手時に「どこまでを1本目の PR にするか」を10分だけ相談する、 2日以上かかりそうなタスクは中間で見せてもらう、 draft PR を早めに立ててもらう——いずれもコストはわずかです。

同じ指摘を繰り返している時

3回同じ指摘を書いたら、それは相手の注意力の問題ではなく、仕組みの不在です。 人間が毎回気をつけて防ぐものは、いずれ必ず漏れます。新人でなくても漏れます。

機械に任せられないか、先に検討してください。

機械化できないもの（設計の粒度、命名の妥当性、ドメイン知識）だけが、 人間のレビューで扱う価値のある領域です。

ケーススタディ: 良かれと思ったレビューが止まった話

ある新人の最初の PR に、レビュアーは32件のコメントを書きました。 すべて技術的には正しく、悪意も皮肉もありませんでした。ラベルはありませんでした。

起きたことは次のとおりです。

• 新人は32件すべてを必須と受け取り、修正に3日かけた
• 変数名の指摘に半日かけた一方、最も重要だった「認可チェックの抜け」は 32件のうちの1件として、他と同じ重みで処理された
• 2周目のレビューが4日後に返り、その間、本人は次のタスクに着手できなかった
• 3本目以降、PR の提出間隔が空くようになった。出すのが怖くなったため

翌月、チームは3つだけ変えました。 ラベルの導入、一次反応1営業日、そして「1つの PR で must: は5件まで、 それ以上あるなら通話する」。

指摘の質は落ちていません。落としたのは量と待ち時間だけです。 それだけで、同じ新人の PR 提出頻度は元に戻りました。

この章のまとめ

• 速さが最優先。1営業日以内の一次反応を決める。読めない時は「読めない」と返すだけでも待ち状態は解消する
• 指摘にはなぜダメかを書く。結論だけでは応用が効かず、同じ指摘を繰り返すことになる
• 主語を人ではなくコードにする。「あなた」が主語になった時点で技術指摘ではなくなる
• 「なんで〜したんですか」は詰問に読まれる。自分の推測を先に書いてから確認する
• 良かった点を具体的に書く。褒めるのは成果物より判断
• must: / should: / nit: / imo: / q: / praise: で強さを明示する。ラベルがないと全部が必須の駄目出しになる
• 承認を止めてよいのは壊れる・危険・戻せない・設計が大きく違うの4つだけ。「自分ならこう書く」は理由にならない
• 指摘は絞る。根本の1件を先頭に置き、同種の指摘は1箇所にまとめる
• 同じ論点で3往復したら通話する。決まったことは PR に書き戻す
• PR が大きい時は、分割の依頼だけでなくどこで切るかを教える
• 同じ指摘が3回続いたら、注意ではなく lint・CI・テンプレートで機械に任せる