コードレビューの受け方・出し方

解説

このレッスンで分かること

• コードレビューが人の評価ではなくコードの検査であること
• must と nits を区別した指摘の書き方
• 指摘を受けた側の返し方と、直さない場合の伝え方

レビューは人ではなくコードを見る

プルリクエストを出すと、次はレビューです。ここで多くの人が最初につまずくのは、技術ではなく気持ちの部分です。指摘されると自分が否定された気がして、身構えてしまいます。

前提を 1 つそろえておきます。コードレビューが検査しているのはコードであって、書いた人ではありません。同じコードなら、誰が書いても同じ指摘が付きます。10 年書いている人のコードにも普通に指摘は付きますし、それで評価が下がることはありません。

この前提は、指摘を書く側の言葉づかいで守られます。次の 2 つを比べてください。

下は同じことを言っていますが、主語がコードです。読んだ側は防御的にならず、素直に条件式を見直せます。主語をコードに置く、これがレビューコメントの基本です。

解説

「なぜこうしたんですか」は、日本語だと詰問に聞こえやすい言い回しです。「この方法を選んだ理由を教えてもらえますか」と書けば、同じ情報を安全に引き出せます。

指摘には強さのラベルを付ける

レビューコメントで一番よくある事故は、書いた側が「参考までに」と思っていた提案を、受けた側が「必ず直せ」と受け取ることです。これは強さが伝わっていないことが原因なので、ラベルで解決します。

多くの現場で使われるのが次の 3 段階です。

nits は nitpick（細かいことをつつく）の略です。imo は in my opinion の略です。読み方は「ニッツ」「アイエムオー」で、そのまま英字で書くのが慣習です。

チームによっては ask（質問）や good（良かった点）を足します。大事なのは種類の多さではなく、必須なのかそうでないのかが 1 行目で分かることです。

実際のコメント例

must の例です。

やっていることは 3 つです。何が問題か、どういう条件で起きるか、どうなってほしいか。この 3 点が入っていれば、受けた側は考え直す必要がなくすぐ直せます。

imo の例です。

提案であることと、今やらなくてもいいことの両方が書いてあります。相手が今回の範囲を守る判断をしても角が立ちません。

nits の例です。

そして、良かった点も書きます。

指摘だけが並ぶレビューは、受ける側にとって消耗する時間になります。良い箇所を 1 つ書くだけで空気が変わりますし、何がチームで良いとされるかの共有にもなります。

解説

コメントに「絶対」「ありえない」のような強い語を使わないでください。強さはラベルで表現します。語彙で強めると、内容が正しくても議論が止まります。

何を見るか

レビューで見る順番を決めておくと、慣れないうちも見落としが減ります。上から順に、重要度の高い順です。

1. 動くか — 境界値、空の場合、エラー時の扱い
2. 壊していないか — 既存の機能への影響、消し忘れ
3. 危なくないか — パスワードや鍵の直書き、入力値の検証漏れ
4. 読めるか — 命名、関数の長さ、条件の入れ子
5. 好み — 書式、細かい書き方

上 3 つが must、4 つ目が imo、5 つ目が nits になりやすい、という対応で考えると分かりやすくなります。

書式については、そもそも人間が見ないのが理想です。Prettier や ESLint のような自動整形の道具をチームでそろえておけば、5 番目の議論は消えます。この考え方は チームでのコンフリクトを減らす でも扱います。

差分だけ見ても分からないこと

Files changed タブに出るのは、変わった行とその前後だけです。ここだけを見ていると気づけない種類の問題があります。

1 つ目は、消えたコードの影響です。不要に見えた関数が削除されているとき、本当にどこからも呼ばれていないかは差分からは分かりません。気になったら、リポジトリ内をその名前で検索してみてください。GitHub の画面でも、キーボードの t を押すとファイル検索が開きます。

2 つ目は、足りない変更です。新しい設定項目が追加されているのに、README や環境変数の例に反映されていない。テストが 1 つも足されていない。差分に「無いもの」は目に入らないので、意識して探す必要があります。

3 つ目は、全体の中での位置づけです。この 30 行が、既存の似た仕組みと重複していないか。これは差分ではなくリポジトリ全体を知っていないと分かりません。だからこそ、そのコードの周辺に詳しい人が 1 人はレビューに入るべきです。

大きな変更のときは、ブランチを手元に持ってきて実際に動かすのが確実です。

gh pr checkout は、その PR のブランチを手元に取ってきて切り替えるコマンドです。番号は PR の番号です。

受け取る側の返し方

次は指摘を受けた側です。ここでも型があります。

指摘に納得して直したときは、直したことが分かるように返します。

コミットのハッシュを添えると、レビュアーはどの変更で対応したかをすぐ追えます。GitHub の画面では、コメントに対して Resolve conversation ボタンを押して会話を閉じます。ただし自分の判断だけで閉じずに、修正内容を返信してから閉じるのが安全です。

直さない選択をすることもあります。そのときは黙って無視せず、理由を書きます。

判断とその根拠、そして今後どうするかが書いてあります。これなら提案した側も納得できます。レビューは全部従うゲームではなく、より良い判断に近づけるための対話です。

分からないときは、分からないと書きます。

分かったふりをして的外れな修正をすると、往復が増えます。

解説

修正コミットを積むときは、--amend で既存のコミットを書き換えるより、新しいコミットを足すほうがレビュアーに親切です。前回のレビュー以降の差分だけを追えるからです。--amend の性質は 直前のコミットを直す（commit --amend） で扱っています。

GitHub の画面でのレビュー操作

GitHub でレビューするときは、Files changed タブを開き、コメントしたい行の左端に出る青い + ボタンを押します。複数行にまたがるコメントは、ドラッグで範囲を選んでから押します。

コメントを書いたあとに出るボタンは 2 種類あります。

指摘が複数あるときは Start a review を使ってください。1 件ずつ送ると、相手に通知が何度も飛び、そのたびに手を止めさせてしまいます。書き終わったら右上の Finish your review から全体コメントを添えて送ります。

送信時に選ぶ 3 つの選択肢も意味が違います。

• Comment — 意見だけ。可否の判断はしない
• Approve — マージしてよい
• Request changes — 直してほしい箇所がある

must が 1 つでもあるなら Request changes、nits だけなら Approve に「直せそうなら直してください」と添える、という使い分けが実務的です。

よくある失敗

ここで実際にありがちな失敗を 1 つ挙げます。レビューを頼まれた人が、丁寧にやろうとして 40 件のコメントを一度に返すケースです。

内訳を見ると、must が 2 件で、残り 38 件は変数名や書式の好みです。受け取った側は、どれが本当に直すべきなのか判別できず、全部直そうとして 2 日かかります。しかも本題である 2 件の不具合は、38 件の中に埋もれて後回しになります。

この失敗の原因は、指摘の量ではなく優先度の欠落です。防ぐ方法は 2 つあります。

1 つ目は、全体コメントに要約を書くことです。

これだけで受け取る側の負担は劇的に下がります。

2 つ目は、書式の指摘を人がやめることです。命名規則や書式に関する nits が毎回大量に出るなら、それは lint の設定で解決すべき問題です。人間が同じ指摘を 3 回書いたら、道具に移す合図だと考えてください。

解説

レビュー依頼が来てから 1 営業日以上放置すると、書いた本人は内容を忘れます。全部見る時間が取れないときは、「今日は前半だけ見ました。残りは明日見ます」と部分的に返すほうが、黙って溜めるより価値があります。

レビューを速く回す

レビューが遅いチームでは、たいていコードの問題ではなく流れの問題が起きています。回すための工夫を 3 つ挙げます。

出す前に自分で読む。 Files changed タブを自分で開いて、上から通して読んでください。デバッグ用の出力の消し忘れ、コメントアウトしたままの旧コード、無関係なファイルの混入は、この 5 分でほとんど見つかります。

差分を小さく保つ。 レビューにかかる時間は差分の量に比例しません。ある大きさを超えると、急に読まれなくなります。粒度の考え方は プルリクエストを出す で扱いました。

依頼を具体的にする。 チャットで頼むときは、URL と一緒に見てほしい観点を添えます。

急ぎかどうかを書いてあるだけで、相手は自分の予定に組み込みやすくなります。