未来を形作るテクノロジーの深掘り記事。

レビューの層が増えるほど、チームは遅くなる

コードレビューを増やしても、コード品質は上がらない。過剰なレビュー工程がボトルネックを生み、開発者を疲弊させ、逆に品質を下げる理由を解説。

長い行列の回転スタンプと検問ゲートの後ろに挟まった、小さな紙の巻物

スタートアップが本番環境にバグを出荷してしまう。経営陣の対応は、コードレビューを必須にすることだ。次はバグがすり抜ける。では二人のレビュアーを必須にしよう。次はセキュリティインシデント。セキュリティレビューの工程を追加する。次は設計の不整合。設計レビューを追加する。2年もすると、どんなに小さな変更でも4段階のレビューを通るようになり、マージまでに3日かかる。かつては毎日リリースしていた開発者は、今やコードを書くより、レビューに時間を費やしている。

このパターンは組織行動の法則と言えるほどありふれている。インシデントが起きるたびにレビューの層が一つ増え、取り除かれることは決してない。その結果、直近のインシデントを防ぐことに最適化されたレビュープロセスは、将来のあらゆる前進を妨げることになる。

レビューキューの数学

各レビューの層は、単に足し算になるのではなく掛け算になる。1回のレビューに平均4時間かかるとしよう。これはレビュー自体の20分ではなく、PRがレビュアーの手に回るまでキューで待つ時間を含んだ数字だ。その場合、直列に2回レビューすると8時間、3回なら12時間、4回なら16時間かかる。

しかも事態はこれより悪い。コンテキストスイッチがあるからだ。開発者はPRを出した後、別の作業に取りかかる。数時間後にレビューのフィードバックが届くと、元の作業に戻り、コンテキストを読み直し、指摘に対応して再提出し、また待つことになる。レビューの往復ごとに、キューの待ち時間に加えて30〜60分のコンテキストスイッチのオーバーヘッドがかかる。

さらに連鎖的な影響もある。レビュアーAが修正を求め、開発者が対応して再提出する。するとまだPRを見ていないレビュアーBが、今度は別の修正を求める。開発者がそれにも対応する。ところがレビュアーAは指摘が反映されたか確認するために再レビューが必要で、すでに別の作業に移っているため、PRはまたキューの後ろに戻ってしまう。

Timeline of a PR through a 3-reviewer process:
Day 1 9:00  — Developer submits PR
Day 1 14:00 — Reviewer A reviews, requests changes
Day 1 15:00 — Developer addresses feedback, resubmits
Day 2 10:00 — Reviewer B reviews, requests different changes
Day 2 11:00 — Developer addresses, resubmits
Day 2 16:00 — Reviewer A re-reviews, approves
Day 3 11:00 — Reviewer C reviews, approves
Day 3 11:30 — Reviewer B re-reviews, approves
Day 3 12:00 — PR merges
Elapsed time: ~3 business days
Actual review time: ~90 minutes total
Actual code change time: ~2 hours
Time waiting in queues: ~22 hours
Queue time is 80% of the total elapsed time.

品質のパラドックス

レビューの層を追加する前提には、レビューを増やせば良いコードになるという考えがある。これはある程度までは正しいが、そこを過ぎると逆転する。

一人の丁寧なレビュアーは、本物の問題を見つけてくれる。ロジックのミス、見落とされたエッジケース、セキュリティ上の問題、API設計の懸念などだ。二人目のレビュアーは、最初の一人が見逃したものを時折(10〜20%程度)拾う。三人目はほとんど、最初の二人が見つけられなかったものを見つけない。レビュアーを一人増やすごとの価値は、急激に下がっていく。

一方で、マージが遅いことによる品質コストは現実に存在するのに、見過ごされている。長期間のブランチはmainから乖離し、リベースが必要になってマージ時のミスを招く。開発者はPRごとのレビューの手間を避けるため、変更をまとめて一つのPRに詰め込むようになる。その結果PRは大きくなり、注意深くレビューするのが難しくなる。レビュアーも疲弊する。キューに15件のPRが溜まっていれば、じっくり読むのではなく、流し読みすることになる。

パラドックスとはこういうことだ。品質を上げるためにレビューの層を足すと、かえって品質を下げることがある。大きなPR、急いだレビュー、古くなったブランチといったインセンティブが生まれ、レビュープロセス自体を損なってしまうからだ。

重いレビュープロセスが実際に防いでいるもの

レビュー工程は、特定のインシデントを理由に正当化されることが多い。「誰もコードをレビューしなかったから、バグを出荷してしまった」というように。しかし、「レビューがあれば特定のバグを防げたか」という問いと、「レビュー必須化が全体の結果を改善するか」という問いは別物だ。

コードレビューの有効性に関する研究では、レビューがおよそ欠陥の60%を検出すると一貫して報告されている。その大半は命名、フォーマット、明らかなロジックエラーといった表面的な問題だ。アーキテクチャ上の深いバグ、並行処理の問題、セキュリティ脆弱性は、差分だけでなくシステム全体の理解が必要なため、コードレビューではほとんど検出されない。本番障害の原因となるバグは、レビューで捕まらない種類のものが不釣り合いに多い。

本番障害を実際に防ぐのは、テスト、モニタリング、そして迅速にデプロイとロールバックできる能力だ。良いテスト、フィーチャーフラグ、即座のロールバックを備えて素早く出荷するチームは、レビューが4段階あるのに結合テストがなく、デプロイに1時間以上かかるチームより本番インシデントが少なくなる。

適切なレビューの量

コードレビューには価値がある。PRごとにレビュアーを一人つけ、何を見るべきかを明確にしておく。これが大半のチームにとってのちょうど良いバランスだ。実際にはこんな形になる。

  • レビュアーは二人ではなく一人。 最初のレビュアーが、レビューで得られる指摘の80%を捕まえる。二人目が加える価値はわずかなのに、コストは大きい。複数人レビューは、本当にリスクの高い変更(データベースのマイグレーション、認証の変更、公開APIの変更など)のためにとっておく。
  • レビューキューに時間制限を設ける。 PRが4時間以内にレビューされなければ、それはレビュアーの怠慢ではなくプロセスの失敗だ。チームはレビューの処理能力を優先的に確保するか、レビュー工程で支えきれない数の開発者を抱えていると認める必要がある。
  • 大きなPRではなく小さなPRを。 50行のPRなら10分で丁寧にレビューできる。500行のPRだと30分かけても表面的なレビューになる。時間が少なくても50行のPRのほうが良いレビューを受けられる。PRのサイズ上限(最大200〜300行)を設けることは、レビュアーを増やすよりレビューの質を上げる効果が大きい。
  • 低リスクの変更ではレビューを省く。 設定変更、文言の修正、依存関係のバージョンアップ、テストの追加などは、ビジネスロジックの変更と同じ精査は要らない。「低リスク」のカテゴリを定義し、マージ後のレビューを前提に、セルフマージを認めよう。
  • 機械に任せられることは自動化する。 リンティング、フォーマット、型チェック、テストカバレッジなどは、人間よりも機械のほうが速く一貫して行える。CIチェックで済むことに、レビュアーの注意を使うのはもったいない。

文化の問題

レビューの層を減らすのは難しい。安全性を減らすように感じられるからだ。本番インシデントの直前に「レビューを減らそう」と主張した人間にはなりたくない、と誰もが思う。これは技術の問題ではなく、組織文化の問題だ。

役立つ考え方は、レビューは多くある安全機構の一つにすぎず、その効果には逓減があると捉えることだ。バグを防ぐために4人目のレビュアーを足すのは、盗難を防ぐために南京錠を4個つけるようなものだ。最初の一個がほとんどの仕事をしていて、それ以上は不便さを増やすだけで、安全性はそれに見合うほど増えない。南京錠を4個もつけたりはしないだろう。レビュアーも4人つけてはいけない。

速く出荷し、壊すことの少ないチームは、本当にインシデントを防ぐ仕組みに投資する傾向がある。包括的な自動テスト、段階的リリースのためのフィーチャーフラグ、アラート付きの堅牢なモニタリング、ワンクリックのロールバック、そしてインシデントを責める対象ではなく学びの機会として扱う、非難しない文化などだ。こうした投資は時間とともに積み上がっていくが、レビューの層にはそれがない。

レビューの層を取り除く

チームがレビュー要件を溜め込みすぎているなら、騒ぎを起こさずに減らす方法がある。

まず現状のプロセスを計測することから始めよう。PRを出してからマージされるまでに何時間かかるか。その時間のうち、キューで待つ時間と実際のレビューに使う時間はどれくらいか。任意の時点でレビューキューに何件のPRが溜まっているか。こうした数字でコストが見えるようになる。多くのチームは、平均的なPRのマージに3日かかっていることを知って驚く。

次に実験を行う。1か月間、レビュアーを二人ではなく一人にしてみよう。同じ指標を計測し続ける。インシデント率は変わったか。コード品質(感覚ではなく欠陥率で測る)は変わったか。ほとんどの場合、答えは「インシデントは増えず、品質は変わらず、スループットが大幅に改善した」になる。

目指すのはレビューをゼロにすることではなく、品質を保ちつつスループットを最大化する、必要最小限のレビューを見つけることだ。その最小限は、今チームが行っていることよりほぼ確実に少ない。レビューの層はインシデント対応を通じて積み重なっていくが、プロセスの最適化によって取り除かれることは決してないからだ。優れたソフトウェアを作ることの多くと同じく、答えはプロセスを増やすことではなく、適切なプロセスを選ぶことだ。