伊藤淳一が考える「コードレビューの目的と進め方」

• keyboard_arrow_down はじめに
• keyboard_arrow_down 本記事の対象読者
• keyboard_arrow_down 本記事で説明することと、しないこと
• keyboard_arrow_down 鋭い人「コードレビューはAIにやらせればいいんじゃないの？」
• keyboard_arrow_down コードレビューは何のため？
• keyboard_arrow_down 可読性、保守性、堅牢性を重視する理由
• keyboard_arrow_down コードレビューの進め方
• keyboard_arrow_down 2回目以降のコードレビューの進め方
• keyboard_arrow_down まとめ

みなさん、コードレビューしていますか？

やってます！　という人の中でも「わざわざ誰かからコードレビューのやり方を細かくレクチャーされたことがある人」は、少ないのではないでしょうか？　「なんとなく見よう見まねでやって、ここまで来た」なんていう人も、きっと多いと思います。まあ、筆者もその一人なんですけどね（苦笑）。

というわけで、この記事では、「コードレビューってどうやるの？」「コードレビューするときは、どんな点に着目すればいいの？」といったポイントを解説していきます。

この記事は以下のような読者を想定して書かれています。

• ・コードレビューされる側から、コードレビューする側に最近回った1〜2年目の新人エンジニア

つまり、これまでコードレビューを受けることは何度かあったけど、コードレビューする側に回るのは初めてで、どうコードレビューすればいいか迷っている、という若手を対象にしています。

また、開発チームで新人エンジニアを指導しているリードエンジニアのみなさんも、チーム内で「コードレビューの手順や観点」を議論するための叩き台として本記事の内容をご活用ください。

この記事では前編と後編に分けて、以下のような内容を解説します。

前編

• ・コードレビューの目的
• ・コードレビューの進め方

後編

• ・コードレビュー時に着目すべきポイント
• ・コードレビュー時に役立つテクニックや考え方

注意してほしいのは、この記事では「コードの良し悪しをどう評価するか」というテクニカルな側面にフォーカスしている点です。「相手を怒らせない、うまいコメントの書き方」といったコミュニケーション面のテクニックについては、すでに『伝わるコードレビュー』のような良書があることから、本記事では対象外としています。

近年では「コードレビューはAIにやらせているよ」という現場も増えてきていると思います。もちろん、それ自体は問題ありません。

ただし、ITエンジニアがコードレビューのやり方をよく理解しないままでよいのか、という疑問は残ります。少なくとも、AIが出してきたレビューコメントを読んで、

「うん、適切だね」
「ここはちょっと的外れだな」
「あれっ、この問題が見落とされているぞ？」

というように、その妥当性を評価できるスキルは身につけておくべきではないでしょうか。

そのためにも若手エンジニアはまず、自分でコードレビューする経験をある程度積んでおく必要があります。だからこそ筆者は、この記事を通じて若手エンジニアが自分の頭でコードレビューできるようになるための考え方や観点を伝えたいと考えています。

この記事を読んで基本を学び、コードレビューの経験を積み、自分の中に確固たる「物差し」をつくりましょう。

さて、前置きはこれぐらいにして、いよいよ本編に入りたいと思います。

最初にコードレビューの目的を確認しておきましょう。

コードレビューをするのは、ひとことで言うと「良いコードにするため」です。

では「良いコードとはどんなコードなのか？」というと、これはひとことでまとめるのが難しいですね。コードの良し悪しにはいくつかの観点があります。

• ・可読性＝人間にとって読みやすいコードになっていること
• ・保守性＝変更したり、機能追加したりしやすいコードになっていること
• ・堅牢性＝壊れにくいコード、不具合を起こしにくいコードになっていること
• ・正しさの確認＝仕様通りに動作し、不具合を起こさないこと
• ・セキュリティ＝セキュリティ上の脆弱性がないこと
• ・パフォーマンス＝十分な処理速度が出せること

最初の3つは長期的な観点と言えます。つまり、今後も継続して開発しやすいコードが「良いコード」になります。

後半の3つは短期的な観点です。つまり、そのコードをリリースしたときに問題を起こさないコードが「良いコード」になります。

これら6つの観点に、優先順位は付けられるのでしょうか？―――いえ、付けられません。いずれも重要です。

ですが、現実的にはコードレビューでは最初の3つ（可読性、保守性、堅牢性）が自ずと重視されるのかな、と思います。

なぜ可読性、保守性、堅牢性がコードレビューで重視されるのかというと、コードレビューは文字通り「コード」のレビューであり、動作確認ではないからです。

もちろん、レビューの一環として「実際に動かしてみる」という作業を加えることはありますが、それはオプションです。「レビュアーが必ずやらなければいけないこと」には含まれていません（少なくとも筆者の現場ではそうです）。

• ・不具合がないか
• ・悪意ある攻撃に耐えられるか
• ・十分な速度が出るか

というのは、コードからある程度は読み解けるものの、これらは動かして確認するのが一番確実です。

一方、

• ・読みやすいコードか
• ・変更しやすいコードか
• ・壊れにくいコードか

というのは、まさに目の前にある「コード」に対する評価です。そして、それらはコードを読める人にしか判断できません。

というわけで、「可読性、保守性、堅牢性、正しさ、セキュリティ、パフォーマンス」は、いずれもコードレビューする際には確認すべき観点ですが、後半の3つについては「コードレビューを最後の砦」にするのは危険です。そうではなく、コードレビューとは別に「実際に動かして検証する作業（品質保証、QA）」を設けましょう。

逆に言えば、可読性、保守性、堅牢性に関しては「コードレビューが最後の砦」になるので、レビュアーが責任を持ってその妥当性を判断する必要があります。

コードレビューの目的を理解したら、次は「どんなふうにコードレビューを進めればいいのか」を見ていきましょう。

コードレビューのやり方や進め方は人それぞれだと思いますが、そもそも他のエンジニアがどんなふうにコードレビューしているのか、実際に見たり聞いたりしたことがある人は少ないのではないでしょうか。

そこで、参考までに筆者がどんなふうにコードレビューを進めているのかをここで紹介します。

プルリクエストを開く

コードレビューのトリガーはプルリクエスト（以下PR）です。筆者の現場ではこんな感じでコードレビューが始まります。

• ・PRがレビュー可能な状態になったら、開発チーム内の掲示板（Slackのようなもの）に通知される
• ・自分の手が空いていてまだ誰もレビューに着手していなければ、「私がレビューします」と宣言してレビューを開始する

なお、場合によっては「伊藤さん、コードレビューお願いします」とバイネームでレビュアーが指定されることもあります（筆者がレビュアーとして最適と判断された場合など）。

diffの大きさを確認する

筆者はPRの規模を確認するために、まずdiffの量に注目します。具体的には以下の画像の赤丸部分です。

ここのプラス（+）の値を見て、なんとなくPRの規模感を測ります。この数字を見たときの筆者の感触は以下の通りです。

• ・100未満＝おっ、これならすぐ終わりそう 😄
• ・100〜500＝まあそんなもんでしょ😐
• ・500〜1000＝むむ、ちょっと多いな🤨
• ・1000超＝多すぎ！　なんで分割してくれないんだ!!😫

もちろん、これはあくまで目安でしかありません。コードとは直接関係のない設定ファイルやテキストファイルが大量に更新されてdiffが増えることもありますし、diffが少なくても予想以上にややこしい修正でレビューに苦労するケースもあります。とはいえ、ざっくりとPRの規模感をつかむには、とりあえずこれで十分です。

descriptionなどからどんな変更が入ったのかを予想する

次に、PRのdescriptionやコミットメッセージを眺めて、このPRがどんなPRなのか（不具合修正なのか、機能追加なのか、単なる設定変更なのか、etc.）を把握します。

そして、descriptionの内容やdiffの大きさなどから、「たぶん、こんな変更が入ってるんだろうな」というふうに、変更内容を予想します。

変更内容を予想するのは、実際にコードをレビューする際に次のような判断をするためです。

• ・予想と実際の変更内容が一致
• → 認識が合っているのでそのままapproveできそう
• ・予想と実際の変更内容が異なる
• → 差分を確認し、必要があれば修正を依頼する

実際に変更されたファイルの一覧を確認する

次に“Files changed”のタブを開きます。

タブを開いたらそのままコードレビューに進んでもいいのですが、diffが大きい場合はいったん左側に表示されるファイルリストを確認します。

ファイルリストを確認するのは、どのファイルに手が加えられたのかを事前にざっくり把握するためです。

実際にコードをレビューしていく

さて、ここまできたら、いよいよコードレビューのスタートです。

筆者は基本的に上から下へ順番にコードを見ていきます。Ruby on Railsの場合、アルファベット順に、

• ・controllers
• ・models
• ・views
• ・tests または spec

とファイルが並ぶため、上から下にレビューしていくと、以下のような手順でレビューできます。

• ・controllerの変更点から、modelとviewに入っているであろう変更点を予想する
• ・そのままmodelをレビュー
• ・続けてviewをレビュー
• ・最後に、ここまで見た変更点を網羅するようなテストコードが書かれているかレビュー

場合によっては順番を変えることもある

ただし、diffの内容によっては上から下に見ていく方式だと、修正内容が複雑すぎて「何をやっているのか、さっぱりわからん」と思うときもあります。このあたりはケースバイケースで、その都度コードレビューしやすい順番でコードを読んでいきます。

先にテストコードを確認して、「今回はどういう仕様変更が入ったのか」をテストコードから把握することもあります。また、先に上から下まで、ざーっとコードを眺めて、「どこにどんな変更が入ったのか」をざっくり頭に叩き込んでから、あらためて最初からコードをじっくりレビューしていくこともあります。

気になった部分にコメントを入れていく

コードを読みながら気になった部分や、改善できそうな点をコメントしていきます。

観点としては、前述の通り「可読性、保守性、堅牢性、正しさ、セキュリティ、パフォーマンス」の6つです。これらの観点でコードを見ていったときに、「ん？」と思う点があればコメントを入れます。

なお、具体的にどんなコードにどんなコメントを入れたらいいのか、といったポイントは、後編の記事で紹介する予定です。

CIのテストがパスしているかどうかを確認する

最後に念のため、CIのテストが全部パスしているかどうかを確認します。

もしテストが落ちている場合は「テストが落ちてるけど、大丈夫？」とコメントします。どこでどんな理由で落ちているのかまでは確認しません。それはPRを出した人に確認してもらいます。

このとき、CIのテストがフレーキー（不安定でたまに落ちる状態）だと、しょっちゅう「大丈夫？」というコメントを入れることになってしまいます。この記事のテーマからは逸れてしまいますが、CIのテストを安定化させることも非常に大事です。

レビューが終わったことを連絡する

一通りコメントが終わったら、“Submit review”ボタンをクリックして1回目のレビューコメントを確定します。

その後、必要に応じて掲示板などで、レビューが終わったことをPRの作成者に連絡します。

もちろん、致命的な問題がないなら1回目のレビューでapproveすることもあります。

以上が筆者のコードレビューの進め方です。

さて、先ほどは1回目のコードレビューが完了した時点で説明が終わっていました。

1回でapproveにならなかったPRは、修正後の再レビューが発生します。というわけで、2回目以降のコードレビューの進め方も見ていきましょう。

前回のレビュー以降の変更点を確認する

diffが大きいPRの場合、最初から全部レビューし直すと時間がかかりますし、前回のレビューから日が空くと、どこをどう直してほしいと伝えたのか忘れてしまいます。

そのため、筆者は基本的に「前回のレビュー以降の変更点」だけをレビューするようにしています。

前回入れたコメントの下に以下のような“View changes”ボタンが表示されている場合、このボタンをクリックすれば「前回のレビュー以降の変更点」をまとめて確認できます。

ただし、“File changed”タブを一度開くと、このボタンは表示されなくなります。ボタンが表示されなくなった場合は、以下のような形式で「前回のレビュー以降の変更点を確認するURL」を自作できます。

https://github.com/your-org/your-app/pull/1234/files/abcd567..HEAD

ポイントは以下の通りです。

• ・前回コメントしたときの最後のコミットIDをコピーする
• ・例: abcd567
• ・PRのURLのうしろに /files/上でコピーしたID..HEADを付ける
• ・例: /files/abcd567..HEAD

このほか、“all commits”を開いて、差分を見たいコミットの始点と終点を「シフト＋クリック」する方法もあります。

approve前に最終チェックする

ここでは「前回のレビュー以降の変更点」に大きな問題がなかったと仮定します。

であれば、指摘事項がすべて解消されたということで、approve!……してもいいのですが、念のため最終チェックをしておきましょう。

筆者は以下のようなポイントを最終チェックします。

• ・あらためて全体を再レビューする
• → 全体を見直すと、今まで気付いていなかった問題点に気付く場合がある
• ・コメントの対応漏れがないかチェックする
• → たまに以前コメントした指摘事項が対応されていないケースがある
• ・AIのレビューコメントをチェックする
• → PRの作成者が「対応不要」としていても、レビュアー視点で見ると「いや、これは対応した方がいいんじゃないか？」と思うコメントがたまに見つかる
• ・CIのテストが全パスしていることを確認する
• → 1回目のレビューでも確認しているが、念のためapprove直前にも確認しておく

これ以上ツッコミどころがなければapprove!

再レビューして、「これ以上ツッコミどころがないな」と思えたら、それがレビュー完了の合図です。

approveを選択して“Submit review”ボタンをクリックしましょう。どうもお疲れ様でした！

というわけで、この記事では「コードレビューの目的」や「コードレビューの進め方」をまとめてみました。

なお、本記事で解説した内容は、あくまで筆者の考え方の一例です。そのため、開発の現場によって異なる部分があって当然です。むしろ、開発現場によって異なる部分があって当然です。もしご自身の開発チームの考え方にそぐわない部分があれば「この記事にはこう書いてあるけど、ウチのチームではこうしようね」と、チーム内の意識を揃えておきましょう。

さて、続く後編では以下のような、より実践的な内容をお伝えしていきます。

• ・コードレビュー時に着目すべきポイント
• ・コードレビュー時に役立つテクニックや考え方

次回もどうぞお楽しみに！