2.5 コードレビューと協働
概要と動機
コードレビューとは、変更がマージされる前に、作者以外の誰かがそれを検査する実践です。ソフトウェア組織が持つ、品質と知識共有のための最もてこの効く活動の一つであり、大きなチームにとっては、調整と文化の主要な仕組みでもあります。レビューは、欠陥を捉え、コードベースの知識を広め、標準を徹底し、エンジニアを育てますが、それはうまくやったときだけです。まずくやれば、ボトルネックや摩擦の源、あるいは偽りの保証を与える形だけの承認になります。
大きなチームでは、レビューは個人の仕事が集団のオーナーシップと出会う場です。そうでなければ個別に働くエンジニア同士の、主な接点であることが多く、その規範が組織全体の協働の仕方を形づくります。レビューは知識を広め、システムのどの部分も一人しか理解していない状態をなくします。これは、大きく長寿命のシステムを悩ませる、知識が少数の人に偏る危険であるバスファクターのリスクを減らします。また、誰が何を変更し、誰が承認したかの監査証跡も作ります。
企業や政府の文脈では、レビューにはしばしばコンプライアンスの側面があります。職務の分離(一人の人が機微な変更の全体を管理しない)、必須の承認、トレーサビリティは、しばしば求められる統制です。機微なシステムに触れる変更は、特定の役割によるレビューを必要とするかもしれず、レビューの記録は監査の証拠になります。課題は、レビューを儀式にせず、速く建設的に保ちながら、これらの統制を満たすことです。
主要原則
- 変更を改善し知識を共有するためにレビューするのであって、見せつけるためではありません。
- 小さな変更はより良いレビューを受けるので、プルリクエスト(PR)を焦点を絞り、適度な大きさに保ちます。
- レビューの遅延はチーム全体のコストです。速い折り返しは全員を動かし続けます。
- 機械的なこと(スタイル、テスト、セキュリティスキャン)を自動化し、人間が設計と正しさをレビューできるようにします。
- ブロックする問題を、提案や好みと分け、どちらがどちらかを明示します。
- 批判するのはコードであり、人ではありません。フィードバックの規範が、レビューが信頼を築くか蝕むかを決めます。
- 変更をレビューしやすくする責任は、作者にあります。
推奨事項
プルリクエストを小さく、よく説明されたものにする
各変更を単一の論理的な関心事に絞り、注意深くレビューできる小ささに保ちます。大きなPRは浅いレビューを受けます。何が変わり、なぜ、どう検証したかの明確な説明を与え、レビュアーが文脈を持てるようにします。機械的なリファクタリングと振る舞いの変更を別々のPRに分け、それぞれを推論しやすくします。良い説明は、レビューの質に対する作者の最も重要な貢献です。
レビューの標準とチェックリストを確立する
レビュアーが何を見るべきかを明記します。正しさ、設計への適合、テストの十分さ、セキュリティへの影響、可読性、標準の遵守。軽いチェックリストは、レビューを一貫させ、重要な次元が抜け落ちるのを防ぎますが、レビューをチェックボックス作業にはしません。何がレビューを必要とするか、誰が承認できるか、機微な領域に必要な役割ベースの承認を定義します。
レビューの遅延の規範を設定し、監視する
目標の折り返し、たとえば一営業日以内の応答に合意し、レビューを最後に詰め込むものではなく、一日の第一級の部分にします。長いレビューの待ち行列はデリバリーを止め、エンジニアを、大きくまとめられた変更へと誘います。最初のレビューまでの時間とマージまでの時間を監視し、持続する遅延を、個人の欠点ではなく、直すべきプロセスの問題として扱います。
機械的なことをすべて自動化する
書式、リンティング、テスト、セキュリティと依存関係のスキャンを継続的インテグレーション(CI)で実行し、レビュアーがそれらに注意を使わないようにします。人間のレビューは、機械が判断できないことのために取っておきます。設計が正しいか、アプローチがシステムに合っているか、テストが意味のあるものか、コードが後でも意味をなすか。
合う所でペアプログラミングとモブプログラミングを使う
複雑または高リスクな仕事、オンボーディング、知識の伝達には、二人のエンジニアが一つの作業台で一緒にコードを書くペアプログラミングを使います。それは継続的なレビューであり、しばしば別のレビューのステップを不要にします。重要な設計判断や、厄介な領域の知識をチーム全体に広めるために、チーム全体が一つのタスクに同時に取り組むモブプログラミングを使います。これらは非同期のレビューの補完として、文脈によって選ぶものであり、あらゆる所で義務づける代替ではありません。
自動化されたAI支援のレビューを慎重に採用する
自動のレビューツールとAIアシスタントを使って、一般的な問題を捉え、改善を提案し、レビュアーの負荷を軽くしますが、その出力を権威ではなく入力として扱います。AIレビューは表面的な問題と一貫性には優れ、深い設計判断とシステムの文脈には劣ります。特にセキュリティに敏感な変更やコンプライアンスに関わる変更では、あらゆる承認に責任を持つ人間を置いてください。
建設的なフィードバックの規範を設定する
フィードバックを、具体的で、親切で、コードに焦点を当てたものに保つ規範を設定します。レビュアーが、命令を出すのではなく質問し、依頼の背後にある理由を説明し、良い仕事をほめるよう促します。ブロックする懸念と任意の提案を明確に印付けします(たとえば、ブロックしないメモに接頭辞を付ける)。これらの規範が、レビューがチームを強くするか、恨みを育てるかを決めます。
トレードオフ: 長所と短所
| アプローチ | 長所 | 短所 |
|---|---|---|
| 非同期のPRレビュー | 柔軟。記録が残る。タイムゾーンをまたいでスケールする | 遅延。ニュアンスが失われる。対立的に感じられうる |
| ペアプログラミング | 継続的なレビュー。速い知識の伝達。高品質 | 一つのタスクに二人。疲れる。調整が難しい |
| モブプログラミング | チーム全体の足並み。深い知識が広がる | 合計では高価。日常の仕事には向かない |
| 必須の複数レビュアー | 強い保証。コンプライアンスに適する | 遅い。責任が拡散する。待ち行列の圧力 |
| AI支援のレビュー | 一般的な問題に速く疲れ知らず。負荷を減らす | システムの文脈を見逃す。過信すると偽りの確信 |
中心的な緊張は、徹底さと速度です。深いレビューはより多くを捉えますが、デリバリーを遅くし、作者を苛立たせえます。速いレビューは流れを保ちますが、表面的になるリスクがあります。進む道は、レビューの深さを変更のリスクに合わせ、些末な変更には軽いレビューを、リスクの高い変更には深いレビューを与え、機械的な仕事を自動化して、人間の労力が重要な所に集中するようにすることです。
チームで議論すべき問い
一つのプルリクエストにとって大きすぎるとは何で、機械的なリファクタリングを振る舞いの変更から分けていますか。 本章は、大きなPRは浅いレビューを受けること、レビューしやすさは作者が負うことを明言し、リファクタリングと振る舞いの変更を分けて、それぞれを推論しやすくするよう求めています。大きなチームでは、巨大なPRは形だけの承認を保証し、本物の欠陥を通り抜けさせながら偽りの保証を与えます。証拠を持ち込んでください。PRの大きさの分布と、差分が大きくなるにつれてレビューの深さがどう下がるか。実用的な大きさの規範と、純粋なリファクタリングをロジックの変更とは別に着地させる習慣に合意し、レビュアーが各変更を実際に頭に収められるようにします。その一つの規律が、その後のあらゆるレビューの質を引き上げます。
ブロックする異議と任意の提案をどう区別し、その規約は実際に使われていますか。 本章は、ブロックする問題を好みと分け、どちらがどちらかを明示するよう求め、好みでブロックすることを腐食的なアンチパターンとして指摘しています。共有の規約がなければ、レビュアーのスタイルの意見が必須の変更と読まれ、恨みを育て、チーム全体のデリバリーを遅らせます。具体的なシグナルとして、最近のレビューで、好みがマージを止めた例を持ち込んでください。ブロックしないメモに印を付ける接頭辞のような軽いマーカーを採用し、作者が何が変わらねばならず、何が提案かをすぐ知れるようにします。それが、レビューを好みではなく、正しさと設計に集中させます。
セキュリティに敏感なコードやコンプライアンスに関わるコードの変更を誰が承認しなければならず、そのルーティングはどう徹底されますか。 本章は、役割ベースの承認、コードオーナーシップのルール、一人の人が機微な変更の全体を管理しない職務の分離を述べ、承認は監査の証拠として記録されます。企業や政府の設定では、これらは求められる統制であり、リスクは、それらが飛ばされるか、デリバリーを凍結させるボトルネックになることです。シグナルを持ち込んでください。どのモジュールが機微で、オーナーシップのルールが現在、それらの変更を適切な承認者に自動的にルーティングしているか。ルーティングをコードオーナーシップの設定に符号化し、自動チェックと小さな変更と組み合わせて、人間の門番の待ち行列なしに統制が満たされるようにします。監査のときにギャップを発見するのではなく、意図してこれを決めてください。
実際に合意したレビュー遅延の目標は何で、それを測定し徹底していますか。それとも願望にすぎませんか。 本章はレビューの遅延をチーム全体のコストとして扱い、最初のレビューまでの時間とマージまでの時間を監視し、持続する遅延を個人の欠点ではなくプロセスの問題として扱うよう求めています。大きなチームでは、所有者のいないレビューの待ち行列が全員に静かに課税します。作者は待ちを避けるためにより大きな変更をまとめ、それらの変更はより浅いレビューを受け、デリバリーのリードタイムは、特定の犯人もなく上昇していきます。相反する考慮は、厳しい遅延の目標がレビュアーに流し読みを促しうることで、速度と深さは、盲目的に交換するのではなく、釣り合わせなければなりません。証拠を持ち込んでください。最初のレビューまでの時間の現在の分布、チームや変更の大きさによる違い、レビューが最も長く置かれる場所。企業や政府の設定では、目標を、リーダーシップがすでに追跡しているフロー指標に結びつけてください。遅延の規範のない必須の複数レビュアーの統制は、デリバリーを凍結させるボトルネックになり、人々に統制全体を迂回するよう誘うからです。
どの種類の変更で自動化されたAI支援のレビューを信頼し、どこで人間が責任を持ち続けなければなりませんか。 本章は、AIレビューの出力を権威ではなく入力として扱うよう述べています。表面的な問題と一貫性には強く、深い設計判断とシステムの文脈には弱く、あらゆる承認に責任を持つ人間がいます。明示的な境界がなければ、大きなチームは過信に流れ、緑のボットのコメントが合格したレビューと読まれ、本物の設計とセキュリティのリスクが偽りの確信のもとで滑り込みます。相反する引力は、AIレビューが確かに負荷を軽くし、一般的な欠陥を疲れ知らずに捉えるので、禁じればてこを無駄にすることです。証拠を持ち込んでください。自動の提案が本物の問題を捉えた所、雑音を生んだ所、そして機械に単独で承認させない変更の種類(セキュリティに敏感、コンプライアンスに関わる、アーキテクチャ上のもの)。企業や政府の仕事では、AIアシスタントが関わっていたとき、承認の責任を誰が負うかを名指ししてください。監査は誰が変更をレビューしたかを問い、「ツールがやった」は規制当局が受け入れる答えではないからです。
ペアリングやモブが非同期のレビューに取って代わるべきなのはどこで、バスファクターのリスクを減らすためにレビューをどう意図して使いますか。 本章はペアとモブのプログラミングを、文脈によって選ぶ継続的なレビューとして枠づけ、レビューを、システムのどの部分も一人しか理解していない状態をなくすように知識を広める仕組みと名指ししています。暗黙のままにすれば知識は集中します。同じ専門家がサブシステムへのあらゆる変更をレビューし、誰も異議を唱えられないためレビューが形だけの承認になり、バスファクターのリスクは、システムが最も重要な所でまさに増します。相反する考慮はコストで、モブはチーム全体の時間を使い、ペアリングは二人のエンジニアを拘束するため、あらゆる所で義務づけることはできません。証拠を持ち込んでください。信頼できるレビュアーが一人しかいないモジュール、オンボーディングが停滞する所、厄介な領域がコメントのスレッドよりライブのセッションから恩恵を受ける所。大きな、あるいは公的な組織では、意図した知識の拡散をリスク管理として扱ってください。重要な部分が一人に依存する長寿命のシステムは、単なる人員配置の不便ではなく、運用と継続性の負債だからです。
セクター別の視点
スタートアップ。 3、4人のエンジニアでは、レビューを軽く保ってください。小さなプルリクエストへの一人の同僚の承認、CIでの機械的なチェック、そしてマージを止める必須の二人目のレビュアーはなし。本当の目標はコンプライアンスというより、システムのそれぞれの部分を一人より多くの人が理解していることなので、リスクの高い部分ではペアを組み、それをオンボーディングとして扱います。すぐに卒業する重いコードオーナーシップのルーティングは作らないでください。小さくよく説明された変更という共有の規範が、ほとんどコストなしに利益の大半を買います。
小規模事業者。 レビューツールの専門家がいる可能性は低いので、独自の自動化を作るのではなく、ホスティングのプラットフォーム(たとえばマネージドなGitサービス)が最初から提供するものに頼ってください。リンティング、テスト、セキュリティスキャンの統合は、保守するのではなく買い、数少ないエンジニアが乏しいレビューの時間を設計と正しさに使えるようにします。単純なルールを一つ保ちます。すべての変更に、もう一組の目を。そして保守する人がいないプロセスを加えるのに抵抗してください。
大企業。 課題は多数のチームにわたる一貫性です。共有の標準、機微な変更を適切な承認者にルーティングするコードオーナーシップのルール、監査の証拠として記録される役割ベースの承認。機械的なチェックを組織全体で自動化し、人間のレビューが設計に集中するようにし、必須の複数レビュアーの統制が静かにボトルネックにならないよう、レビューの遅延をフロー指標として追跡します。文書化された方針でレビューの深さを変更のリスクに合わせ、些末な変更は速いままにし、高リスクのものには職務の分離とより深い精査を与えます。
政府。 変更管理はしばしば必須です。本番へのあらゆる変更を、作者以外の誰かがレビューして承認し、職務の分離の要件を満たすために、記録を監査の証拠として保持します。誰が書き、誰が承認し、どのチェックが通ったかの透明で追跡可能な跡を好み、統制がデリバリーを凍結させないよう、自動化と小さく頻繁な変更に投資します。レビューツールを調達するときは、エクスポート可能な監査ログを求め、ロックインを避けてください。証拠はどの単一のベンダーより長く残り、公衆の精査に耐えなければならないからです。
事例
スタートアップ。 4人のエンジニアのスタートアップは、すべてのプルリクエストを小さく保ち、マージ前に一人の同僚の承認を求めます。コンプライアンスのためというより、システムのある部分を理解しているのが一人だけという状況をなくすためです。CIがフォーマッターとテストを実行するので、人間は乏しいレビューの時間を、空白ではなく設計と正しさに使います。チームが決済フローの厄介な部分にぶつかったとき、二人は非同期のコメントをやり取りする代わりにペアを組み、それは最も新しい採用者のオンボーディングも兼ねます。
大企業。 大手のソフトウェア会社は、すべての変更に少なくとも一つの承認レビューを求め、コードオーナーシップのルールで特定されたセキュリティに敏感なモジュールへの変更には、二つ目の承認を求めます。CIがスタイルとテストのチェックをすべて扱うので、レビュアーは設計と正しさに集中します。チームは最初のレビューまでの時間を追跡し、中央値の上昇を、作業負荷を再均衡させる合図として扱います。新しいエンジニアはペアリングでオンボードされ、独立して貢献するまでの道のりが短くなります。
政府。 厳格な変更管理の要件のもとで運用される国の機関は、本番へのあらゆる変更を、作者以外の誰かがレビューして承認することを義務づけ、承認は監査のために記録されます。この統制がボトルネックにならないよう、機関は自動チェックと小さく頻繁な変更に投資し、その日のうちにレビューに応答する規範を設けます。誰が書き、誰が承認し、どのチェックが通ったかというレビューの跡は、各リリースのコンプライアンスの証拠の一部になり、デリバリーを凍結させずに、職務の分離の要件を満たします。
ビジネスケース: 動機、ROI、TCO
コードレビューは、三つの通貨で元を取ります。本番の前に捉えられる欠陥、チーム全体に広がる知識、そして時間をかけて自動的に守られる標準です。レビューで欠陥を捉えることは、本番で捉えるよりはるかに安く、知識共有の利益は、誰かが去るとき組織に大きな損害を与えうる、特定の人物へ依存するリスクを減らします。レビューはまた、成長するチームを一貫させる文化の伝達の仕組みでもあります。
レビューのコストはエンジニアの時間と多少の遅延で、どちらも良い実践で管理できます。レビューしない、あるいはまずくレビューすることのコストには、本番の欠陥、サイロ化した知識、一貫しないコード、そして規制された環境では、失敗した監査とコンプライアンスの指摘が含まれます。過度に重いレビューにも本物のコストがあります。長い待ち行列、大きくまとめられた変更、意欲を失ったエンジニア。リーダーシップに論拠を示すには、レビューの実践を変更失敗率、デリバリーのリードタイム、オンボーディングの速さに結びつけ、レビューの遅延を明示的なフロー指標として追跡してください。
アンチパターンと落とし穴
- 形だけの承認: 本物の検査のない承認で、偽りの保証を与え、統制の文言だけを満たすこと。
- 巨大なPR: 流し読みしかできない数千行で、浅いレビューを保証すること。
- 些末な指摘だけのレビュー: 設計と正しさを見逃しながら些事に集中すること。多くの場合、機械的なチェックが自動化されていないため。
- 門番としてのレビュー: 支配を主張したり他者をブロックしたりするためにレビューを使い、協働を毒すること。
- 遅い待ち行列: 何日も置かれ、デリバリーを止め、まとめ作業を促すレビュー。
- AIレビューの過信: 自動の提案を権威あるものとして扱い、リスクの高い変更で人間の判断を捨てること。
- 好みでブロックする: 個人のスタイルの意見を、本物の欠陥と区別せずに必須の変更として提示すること。
成熟度モデル
- レベル1、開始: レビューは場当たり的で反応的です。しばしば飛ばされたり一貫せずに行われたりし、コメントは機械的な問題が占め、フィードバックの規範は設定されておらず、承認の跡は意図ではなく偶発的です。
- レベル2、発展: 基本的なレビューの実践はありますが、チームごとに異なります。レビューが必須の所もあれば遅いか任意の所もあり、自動化は部分的で、プルリクエストの大きさと質は、共有の期待なしに大きく振れます。
- レベル3、標準化: 標準は文書化され、組織全体で徹底されています。小さく焦点を絞ったPR、CIでの自動の書式、リンティング、テスト、セキュリティスキャン、明確なチェックリスト、ブロックと提案の明示的な規約、機微な変更を適切な承認者にルーティングするコードオーナーシップのルール。
- レベル4、管理: レビューはベースラインに対して測定され、制御されます。最初のレビューまでの時間、マージまでの時間、変更のリスクに対するレビューの深さ、欠陥の流出率、変更失敗率が追跡され、持続する遅延はプロセスの問題として扱われ、データが、レビュアーの負荷をどこで再均衡させるか、保証を加えずにデリバリーを遅らせている統制がどこにあるかを導きます。
- レベル5、オーケストレーション: レビューは継続的に改善され、組織全体に統合されています。深さは変更のリスクに適応し、ペアリング、モブ、AI支援は人間が責任を持って意図して使われ、知識の拡散とバスファクターのリスクは意図して管理され、レビューは品質、デリバリーの流れ、オンボーディングを測定可能なほど改善します。
議論のためのアイデア
- あなたのチームにとって適切なレビュー遅延の目標は何で、それを達成するのを何が妨げていますか。
- 官僚主義を加えずに、レビューの深さを変更のリスクに合わせるにはどうしますか。
- あなたの文脈で、ペアリングやモブが非同期のレビューに勝るのはどこですか。
- AI支援のレビューはどれだけ信頼すべきで、どの種類の変更に対してですか。
- チームが成長し多様化するなかで、レビューのフィードバックを建設的に保つにはどうしますか。
- ボトルネックを作らずに、コンプライアンスの承認要件を満たすにはどうしますか。
要点
- プルリクエストを小さくよく説明されたものに保ちます。レビューしやすさは作者が負います。
- 機械的なことを自動化し、人間が設計、正しさ、テストをレビューできるようにします。
- レビューの遅延を、チーム全体のフローのコストとして追跡し管理します。
- レビューの深さを変更のリスクに合わせ、ブロックする問題を好みと区別します。
- ペアリング、モブ、AI支援を、文脈に合う補完として使い、人間に責任を持たせ続けます。
参考文献とさらなる読み物
- Karl Wiegers, Peer Reviews in Software: A Practical Guide
- Google, Engineering Practices: How to Do a Code Review (as a reference exemplar)
- Nicole Forsgren, Jez Humble, Gene Kim, Accelerate: The Science of Lean Software and DevOps
- Kent Beck, Extreme Programming Explained (on pair programming)
- Woody Zuill, writings on mob programming
- Michael Lopp, Managing Humans (on engineering collaboration)