[翻訳] コードレビューについて

この記事は::..: glen.nu :.: ramblings :.: on code review :.::の意訳記事です。@9len氏の許可を受けて投稿しています。誤り・修正などがありましたら、@iwashi86までご連絡いただけますと幸いです。

This article originally appeared in English at :..:: GLEN D SANFORD :.: RAMBLINGS :.: ON CODE REVIEW ::..: and has been translated with @9len’s permission for posting to this blog in Japanese.

この記事は2014年3月に書いている。Twitterでユーザ検索チームを私が率いていたころの話だ。この記事は、コードレビューに関するセオリー・アプローチを体系化することを狙いとしていて、いくつかの基本的なプラクティスの確立を狙っている。あなたのチームの働き方を述べる記事ではなくて、私が重要だと考える”タイムリーなコードレビューは、チームのコード品質と生産性の両方の役立つ”ということを述べる。

TL; DR

コードレビューはおそらく、常に最優先であるべきだ。あなた自身のイベントループにコードレビューを組み込むための最適な方法を、理解しておくこと
レビュー依頼をするときは、内容をレビュワーが読んだ時に嬉しいようにすること

個人的には、チームとして生産性をあげるためにもっとも重要なのは、全てのチームメンバがコードレビューを最優先として意識することだ。私の思いつく、もっとも説得力がある簡潔な理由は以下のとおりなので、強調して述べる：

コードレビューを先延ばしにするのは、スレッド処理をブロックしていることと同義だ

最良のケースでは、コードレビューでブロックされたエンジニアは、何らかの変更について進捗をあげられる。ただし、コンテキストスイッチとメモリ割り当てのコストは支払う。もちろんこれが得意なエンジニアもいる。だけど、変更内容が増えるにつれて、メモリ使用総量が増えて、もしページアウトが発生すると、コンテキストスイッチのコストはかなり高くつく。

最悪のケースでは、コードレビューでブロックされたエンジニアは、進捗0だ。理由は、そのエンジニアに割り当てられた仕事は、そのレビューが終わらないと進められない仕事だからだ。もちろん、私（マネージャ）としては、並列実行できるように仕事を割り振りするが、どうしても不可能な場合もある。そういう場合は、コードレビューの残量が、他の仕事を終わらせるための同期的な障壁になる。

チームとしては、自分たちのゴールを達成するために、互いのメンバに依存しており、完全に互いの仕事を並列化できないこともある。よくあるのは、インフラの欠陥でブロックされたり、他の仕事を待ったりするケースだ。自分たちのコントロールという観点で考えると、何らかの仕事がブロックされているときは、私たちは誰かがコードを書いていること、またはレビューしていることをブロックしていることになる。

これまでに前提は共有したので、次はどうやってこれを実現していくか？という話だ。これについては、可能な限り効率よくするために、レビューする側・レビューされる側の両方でやるべきことがある。

レビューする側

自分が担当しているレビューについて知ること

だれか(またはツール)があなたにお願いした全ての待ち状態のレビューを見つけるシンプルな方法をもつこと。これに関しては、個人的にはレビューボードが特に有用だとは思わなくて、結局古臭いメールを使ってる。どんなメールフィルタやツールを使っても構わないが、担当している分のレビューを素早く見つける方法を確実に持っているように。

イベントループを短くする、可能な限り割り込みに対応する

コードレビューが必要なタスクを素早く見つけられるようになったら、その方法をあなたの仕事のイベントループに組み込んで、イベント起動できるようにすること。理想論で言えば、コードレビューがきたらすぐやるという意味だ。だれしも、仕事を終わらせるためには集中する必要がある。そのため、処理中のコンパイル・テスト・コーディングのループから、コードレビューに移行するためのコストはタダじゃない。でもよく考えてほしい。これらのコンテキストスイッチのコストと、コードレビューでブロックされているときのコンテキストスイッチのコストはどちらが大きいだろうか。今取り組んでいることを終わらせるために数分、コードレビューを遅らせるのは構わない。だけど、数時間や１日遅らせてはダメだ。もし、食事に行く・トイレに行く時間があるなら、あなたのコーディングを再開するまえに、おそらくコードレビューする時間がある。

コードレビューを最優先に！ ※

※ 障害やお客様対応は除く

上に述べた2点と関連しているけど、重要なので明記する。最初に書いたように、コードレビュー待ちが発生するとスレッド実行がブロックされ生産性が落ちる。もしコードがレビューされないということは、あなたは自身の生産性をチームの残り全体の生産性より優先度を高くしているということになる。もし、チーム全員が75%の効率で動いているとして残り25%はコードレビューしているのなら、チームの数人が95%で動いていて後のメンバはアイドル状態/スラッシング状態よりも、良い結果をもたらす。

レビューしても、何にもコメントすることが無いなら、こう言おう！少なくともこんな感じで「LGTM、だけどコードをよく理解していない」とか。重要なポイントは、レビューをはじめるときと終わるときの行き来の時間を最小限にすることだ。そうすると、ブロックされる時間が減る。

毎日コードレビューに時間を費やす、1日中でも

1on1(訳注：マネージャとエンジニアの1対1の会話)でたまに話題にあがる質問は、「どれぐらいの時間をコードレビューに費やしたらいい？」という質問だ。簡潔に応えるなら「必要な時間でいくらでも」になる。最低限、業務の開始と終了時に、コードレビューのバックログをクリアにすべきだ。理想的には開始・終了の間にも、1-2回はバックログをクリアする時間を持つべきだ。完全にバックログを空にするのがここでのゴールだ。そのために1-2時間費やすのは、合理的だと思う。

徹底的に、毎回・毎日

全くコードレビューしない状態より悪い唯一の状態は、本来は分割できるにもかかわらず、複数日にまたがる1回の良いコードレビューをする状態だ。もちろん、整形され注意深く仕上げられたコードを書くのはレビューを依頼する側の役割だ。レビューする側の仕事は、毎回出来る限り徹底的にやることだ。もし、複数リビジョンにレビュー自体がまたがってるなら、いい加減になってるかもしれない。それは第一に、レビュワーが十分に注意を払えてないのが原因だ。これはかなり難しいし、一貫してやらないといけない。もし、レビューサイクルで遅れが出てるとわかったら、正直に告白して謝ろう。

レビューをされる側(レビューをお願いする側)

DOT YOUR I’S AND J’S

レビューを最優先してもらう代わりに、レビュワーには良いコードを送るようにしよう。レビューをお願いする前に、自分で注意深くレビューしよう。変更点に不足が無いように、また不必要なものが無いようにしよう。自分の能力の極限まで、属しているプロジェクトに適したコードを書こう。レビューを投げた後に、すぐに複数回修正するってのはダメだ。これはレビュワーを困らせるから。もし、自分で気に入らないところがあるなら、修正する/やり直すのが良い。

小さく保つ

小さくて、消化しやすいレビューに。可能であれば、大きなリファクタは小さく分割する。変更に対して理由を述べやすいように、また順番に出来るように。（順序性がないとしても分割されてた方がいい）もし、レビューが2つのページ以上に渡るなら（または複数ファイルに渡るなら）、分割できないか自分自身に問うてみよう。もし、全員がコードレビューするのに優れているチームなら、小さな変更をまとめておくメリットはなくて、デメリットしかない。大きなレビューで最も危険な結果は、複数の変更にまたがった変更に対してレビュワーが集中できずに、ほとんどレビューされない・まったくレビューされない事態が起こるという結果だ。

良いタイトル・説明を

タイトルを見れば、レビュー一覧の中から明らかに意味が伝わって分かりやすくなっていること。レビュー依頼に書く説明は、変更に関するストーリーを書くべきだ。例えば、なぜその変更を加えているのか、どういう問題を解決しているのか、どこが変わったのか、どのクラスが導入されたのか、どうやってテストしたのか、という点だ。曖昧なタイトルが連なったコミットになっているべきではない。レビュワーに対して、特に何に気をつけないといけないか、分かりやすくなっているべきだ。レビュワー自身も、どんな観点で見れば分かりやすくなるので、感謝してくれるだろうし、何よりレビューがより効果的・徹底的になる。

すぐにコメントしてACKを

しっかりしたイベントループは、コードがレビューされているときには、さらに重要になる。つまり、レビュワーからコメントがあったら、すぐにACKするのに全力を尽くすこと。またコメントが理解できなかったら、すぐに詳細な説明を求めること。レビュワーがレビューを投稿したら、次のアクションはあなたの番になる。もし、変更点がわかりやすくて小さかったら、新しいレビューをすぐに投稿し、常にボールが回っているようにする（レビュー -> コメントのやり取りが続くように)。早くコメントや欠陥に対応できればできるほど、レビュワーはページインしている状態で、レビュワーとの合意形成に早く到達できる。

終わりに

ここまで読んでくれてありがとう。少なくとも、レビュー依頼が1件は溜まっているはずだよ。