会員事業部の三吉（@sankichi92）です。 クックパッドでは、GitHub Enterprise の Pull Request を使ったコードレビューを広く実施しています。 この記事では、私がコードレビューすることに対する苦手意識をなくすために意識したことを紹介します。
クックパッドでは、テックリードや新卒、インターン、バイトといった肩書きに関係なく、誰もがレビュワー・レビュイーになります。 チームやプロダクトによって開発ルールは少しずつ異なりますが、私の所属する会員事業部では、PR を出したときに GHE やチャットで部内のエンジニアにメンションして、その時にレビューできる人がレビューするという形を取っています。
私は、昨年2017年に新卒入社したのですが、それまでは個人開発や研究用のコードしか書いたことがなく、短期インターンシップを除くチーム開発の経験がありませんでした。 配属当初からコードレビューすることは求められていのですが、はじめの数ヶ月間はレビューすることに対して苦手意識があり、なかなか積極的にレビューに参加することができませんでした。
コードレビューの難しさ
コードレビューに対する苦手意識は、自分のレビュー内容に自信が持てないことによるものでした。 コードレビューは、真剣に取り組もうとすると非常に難しい作業です。 以下、「見るべき項目の多さ」「文脈によるレビュー内容の変化」「開発速度とのトレードオフ」の3点からコードレビューの難しさを見ていきます。
見るべき項目の多さ
コードレビューでチェックするべき点は非常に多いです。 以下に、ざっくりとレビューの観点をカテゴライズしました（順序に深い意味はありません）。
- 挙動（意図どおり動作するか）
- バグ
- セキュリティ
- 可読性
- テスト
- パフォーマンス
- ドキュメンテーション
- 設計 *1
- ……などなど
これらは、それだけで分厚い本が書けるようなカテゴリであり、それぞれについて一般的な原則や社内のルールがたくさんあります。 ちょっとした変更であればともかく、毎回のコードレビューでそれらを厳密に網羅することは不可能で、どこかで妥協する必要があります。
文脈によるレビュー内容の変化
では、どこで妥協するのかというと、それはレビュー対象のコードの文脈に依存します。
極端な例を挙げると、ユーザーテスト用のプロトタイプと、決済周りのデータ処理を行うジョブとでは、どのくらい細かくレビューするべきか変わってきます。 前者は、あくまでプロトタイプであって、テストするのに問題なければ最悪バグが含まれていても構いません。 それに対し、後者にバグが含まれていると、深刻なデータ不整合が起きたり、ユーザーに不利益が出たりすることになります。 後者をレビューするときには、テストの内容やエンバグの可能性の丁寧なチェックが必要です。
他にも、変更頻度の高い箇所であればメンテナビリティを考慮したり、検索などリクエストの多い部分であればパフォーマンスを意識したりと、コードの位置する文脈に依存して、重点を置くべき観点が変わってきます。
開発速度とのトレードオフ
レビュー時どこに重点を置くべきかは、文脈だけでなく、レビューに割ける時間によっても変わります。
コードレビューは思いのほかコストのかかる作業です。 レビューしているあいだレビュワーは他の作業をすることができず、また、レビュイーにも指摘箇所の修正だけでなく、レビューがつくたびに発生するコンテキストスイッチの負担があります。
しかし、コードレビューにはそのコストをかけるだけの価値があります。 私たちがコードを書く目的は、ユーザーに価値を届けることであり、コードレビューの目的も変わりません。 コードレビューは、届ける価値の品質を担保するために必要な作業です。
とはいえ、サービス開発においては、その価値をすばやく届けることも非常に重要です。 レビューに不必要なまでに時間をかけて、高速な開発サイクルを回せなくなっては本末転倒です。 品質と速度とのトレードオフから、レビューにどの程度コストをかけるべきか考える必要があります。
コードレビューをするときに意識していること
以上にみてきたように、コードレビューは、無数の項目について、コードが置かれた文脈から優先順位をつけ、開発速度と品質を最大化するような時間でチェックしていく、というとても困難な作業です。
……と、これが理想のコードレビューかもしれませんが、人間が意識的にできるものではありません。 重要なのは、そういった難しさがあることを知った上で、できる範囲でやる ことです。 とはいえ、コードレビューに慣れない頃は、できる範囲でやったレビューには多くのヌケモレがあるように思えて不安になったり、しっかりレビューしたときには時間をかけ過ぎではないかと不安になったりしていました。 ここからは、そうした不安や、それに起因する苦手意識をなくすために私が行っている工夫を紹介します。
レビューした範囲を明示する
自分のレビューが不十分に感じられたときは、何を見たか、あるいは、何を見られていないかを Review summary などに書くようにしています。
こうすることで、他のレビュワーに重点を置いて見てほしい場所を伝えています。 広く薄く見ただけであれば「ざっと見ました」というコメントでも構いません。
また、自信のないときは、素直に他のレビュワーに依頼します。
重要な定数などについて、仕様と照らし合わせて正しいことを指差し確認するのも、ヌケモレを防ぐのに大事です。
わからなかったら質問する
もちろん、他のレビュワーが現れることを期待できない場合は、ひとりで全範囲をレビューする必要があります。 そういう時に自信を持てない箇所、わからない箇所を見つけたら、わかった気になるまで読むのではなく、質問することが重要です。
次のように、質問に答えることで、実装者がミスや考慮漏れに気づくということも少なくありません。
コードから汲み取ったことを言語化して、その認識であっているか確認するだけでも効果があります。 込み入った内容であれば、実装者と一緒にペアレビューするという方法も有効です。 ペアレビューで理解した内容を PR にコメントするとなお良いです。
コードレビューには、問題点を見つけるだけではなく、そのコードの理解者を増やして属人性をなくす機能もあります。 GHE に残った質問のログが数年後に再び役立つということも珍しくありません。
nits や IMO, MUST といったラベルを利用する
先にも述べたように、サービス開発ではスピードも重要です。 本質的でない修正に時間をかけるよりも、先にリリースした方が良い場合もあります。
とはいえ、レビューしていると、どうしても細かいところが気になってしまうものです。 そうしたときは、[nits] や [IMO] といったラベルをレビューコメントの先頭につけて、修正の判断を実装者にゆだねます*2。
逆に、どうしても修正してほしい場合は [MUST] ラベルをつけます。 すぐに修正できない問題であれば、その PR で修正するのではなく、Issue にして後日修正するのも有効です。
おわりに
以上が、私の意識している「無理をしないコードレビュー」です。 レビューすることに慣れて、日常化すれば特に意識する必要はなくなります。 しかし、1年前の私にとってそれはとても難しいことだったので、当時の私に伝えるつもりでコードレビューするときのコツを書いてみました。
コードレビューする機会が増えて感じるのは、レビュワー側も非常に勉強になるということです。 レビューする時の視点は、単にコードリーディングする時の視点とは違います。 エンバグはないか、テストは必要十分か、などなど普段以上に神経をとがらせて見ることになります。 そして、その視点はそのままコードを書く時にも活かすことができます。
また、今回はコードレビュー「する」ときの工夫に焦点を当てました*3が、「される」ときの工夫も重要です*4。 他にも、クックパッドでは、コードレビューのコストを下げるための自動化等の取り組みも行っています*5。
コードレビューのスタイルは、組織や開発するプロダクトの性質によって変わってくるものだと思いますが、この記事が少しでも参考になれば幸いです。
*1:大きな変更であれば事前に設計レビューを行います。
*2:リポジトリにもよりますが、クックパッドでは実装者がマージを行うことが多いです。
*3:コードレビューすることに関する記事として、他にも たのしくなるコードレビュー があります。
