コードレビューを `small-change` で高速化する
Speeding up code review with `small-change` (lethain.com)
要約
Imprint社では、プルリクエスト(PR)のレビュー時間が長くなるという課題に直面していました。この問題を解決するため、1,000行未満のコード変更、自動レビューボットのP1/P2エラーなし、レビュアーの経験値、影響範囲の限定といった基準を満たすPRを「small-change」と定義し、迅速な承認を可能にする仕組みを導入しました。これにより、レビューのボトルネックを解消し、開発速度と品質の向上を目指しています。
全文翻訳
コードレビューを small-change で高速化する
Imprint社では、平均的なエンジニアが2年前と比較して4倍多くのPRをマージしています。さらに興味深いのは、その平均値の裏に、PRのマージ数が10倍になったチームが2つあることです。あるエンジニアは、2年前の平均的なエンジニアの週あたりのPRマージ数の25倍をマージしており、他にもそれに迫るエンジニアが数名います。これは素晴らしいことで、この傾向が続けば、1年後には平均値が現在のp90(90パーセンタイル)に近づくでしょう。さらに重要なのは、単に多くのものを出荷しているだけでなく、品質も向上していることです。インシデントは半期比で42%減少し、重大なインシデントは78%減少しました。品質と速度のトレードオフという考え方はあまり有用ではないと以前から知っていましたが、それは今日では否定できません。
この道を歩む中で、私たちは(Anthropicのように)プルリクエストレビューという問題に直面しました。2年前、中央値(p50)のプルリクエストレビューにかかる時間は約33分でしたが、現在では約77分かかります。この増加率は、マージされるプルリクエストの数と密接に関連しています。その関連性が非常に高いため、この問題への対応が今週の私の優先事項の一つとなりました。
解決しました!というのは冗談ですが、まだ完全に解決したわけではありません。しかし、私たちの現在の取り組みは興味深いものであり、「small-change」と呼んでいます。
ここで議論されている作業は、Dan、Joy、Daniel、Lenny、Taylor、Ted、Annieをはじめ、Imprintの多くの人々によって形作られ、実行されました。
なぜプルリクエストをレビューするのか?
プルリクエストをレビューする理由は4つあります。
- 作成者とレビュアー間の知識共有 – ソフトウェアエンジニアリングは一般的に見習い期間を通じて学習され、レビューは経験豊富なエンジニアが優れたソフトウェアとは何かを他の人に教えるための主要なメカニズムです。現在のボリュームでは、この知識共有は個々のプルリクエストではなく、プルリクエストのクラスターを対象とするようになる必要があると考えています。
- エラーの防止 – レビューは常にエラーを発見するための高コストな時間でしたが、効果的なCICDと、Potetoのpstackのようなハーネス検証技術が、レビューよりもエラー防止に効果的なメカニズムであると信じています。
- 悪意のある行動の防止 – 私の経験では、注意深いコードレビューであっても、有能な悪意のある行動を見逃すことは一般的にありません。幸いなことに、ほとんどの悪意のある行動は無能なものです。これらの両方の種類の行動は、固定ルール(特定のオブジェクト/クラス/ファイルを変更するにはレビューが必要)、LLMベースのメンバーサポート体験で洗練されたアンチエイバス機構、監査、および監査ログの検査の組み合わせによって最もよく特定できると考えています。
- 文書化されたコントロールにおけるコンプライアンス義務 – これらは変更可能ですが、今日の要件では、リポジトリのサブセットに対する変更を独立した人間がレビューする必要があります。
現代の開発パラダイムでは、これらの各問題に対してより良い解決策があると考えています。しかし、これらの変更は非常に慎重に行いたいと考えています。
small-change による小さな一歩
私たちは、新しいプラクティスを検証するための狭く、閉じられた方法を見つけることによって速く進むことができると信じており、効果的な解決策への反復的なアプローチとして、「small-change」の概念を開発しました。仕組みは以下の通りです。
- プルリクエストが準備完了とマークされると、CICDはそれが
small-changeの対象となるかどうかを判断します。これは、GitHubのチェックがパスしたことで示されます。可視性のためのラベルも存在しますが、ビルドによって管理され、人間が変更できないチェック自体が適格性を決定します。(ラベルは初期フィルタリングに使用されますが、チェックは引き続き独立して検証されます。) small-changeとは、以下のすべての基準を満たすものです。- コード行数が1,000行未満であること。
ai-code-review自動レビューボットからのP1またはP2の検出がないこと。- 作成者がそのサービスまたはアプリケーションのディレクトリに対して10件以上のPRをマージした経験があること。複数のサービスまたはアプリケーションにまたがる変更の場合、作成者はそれらすべてに経験がある必要があります。
N個未満の異なるサービスまたはアプリケーションに影響を与えること。フロントエンドモノレポではNは1、バックエンドモノレポではNは3です。典型的な変更はgRPCサービスと共通のHTTPゲートウェイの両方を変更します。- 共有ライブラリやユーティリティファイルは
small-changeの対象外です。 - 各リポジトリには
.small_changeファイルがあり、動作を調整し続けるためのパスごとのカスタマイズが可能です。例えば、資金移動を扱うサービスは除外できます。
- 私たちのSlack自動化は、プルリクエストへのすべての参照にリアクション(reacji)を付け、プルリクエストの状態を示します。例えば、テストが失敗している場合、承認されたがテストが実行中の場合、またはマージ準備完了の場合などです。
small-changeの基準を満たすプルリクエストを示すsmall-changeリアクションを追加しました。 - 社内のスキルリポジトリに、ハーネス全体に配布された2つのスキルを作成しました。
-
review-small-change:リポジトリのリストを受け取り、オープンでドラフトではないすべてのsmall-changeをレビューし、基準(すべてのチェックがパスし、small-changeチェックを含む、支払いやセキュリティなどの機密トピックに触れていないこと)を満たす場合は承認を提供します。重要なのは、このスキルには承認または無視の2つのオプションしかないことです。PRにコメントすることは決してありません。これは、一部のチームでボトルネックを装ったソリューションとなっている「ミートプロキシレビュー」の拡散を避けるためです。
-
review-pr-cluster:プルリクエストのバッチをレビューし、共通のテーマを特定し、議論や検証の追加の機会を提案します。
-
small-changeによって引き起こされたエラーはシステム全体の失敗であり、システムレベルで解決されなければならないという社会的な期待を設定しました。これは、ai-code-review、検証メカニズム、CICD、ハーネスなどの改善を意味します。
これは、断続的な注意で約2日間で実装およびロールアウトできましたが、おそらく永遠に調整を続けることになるでしょう。
ミートプロキシレビュー
上記の4.1で言及しましたが、ミートプロキシレビューは多くのチームにとって興味深いボトルネックとなっています。これは当初は解決策のように見えますが、事態を悪化させるため、特に難しいボトルネックです。そのパターンは以下の通りです。
- チームはレビューに圧倒される。
- チームはローカルハーネスでプルリクエストをレビューすることを決定する。
- プルリクエストには多くのコメントが付くが、意味のあるものもあれば、そうでないものもある。どれがどれか理解するには調査が必要で、レビュアーの意見を反映しているかどうかもわからない。そのため、レビュアーが同意していない可能性のあるコメントを反証するためにかなりの作業を行うことになる。
- これらのチームは、ハーネス生成コメントに対応するためにハーネスを使用し始め、レビュープロセス全体が学習を促進しない幻想となり、最も重要なことに、他のチームメンバーの関与なしに行うことができる。
ミートプロキシレビューが行われるすべてのケースで、チームは中央集権的なレビューメカニズム、中央集権的なレビューからのフィードバックに対応するハーネスのループ、そして単に承認をスタンプすることに投資することで、より良い結果を得られるでしょう。
fleet: 中央集権的にオーケストレーションされたハーネス
コードレビューのボリュームを削減するためのもう一つのアプローチとして、中央集権的にオーケストレーションされたハーネスであるfleetに焦点を当てます。(これはStripeのMinionsに似ていますが、特にワンショットアプローチには従っていません。)fleetは、独立した評価メカニズムを備えた、クラウドで実行される高度に制御されたハーネスでソフトウェアを実装および検証するために使用され、任意の変更に対して独立したレビュー担当者の適切な数を特定します。その数は、安全と評価された変更に対しては1(例:fleetが記述し、人間が1人レビューする)と低くすることができます。
結果 / 次のステップ / 反響
正直に言うと、有用な結果を示すには時期尚早です。初期の結果では、約10%の変更がsmall-changeの資格を得ています。これらは現在、非常に迅速な承認の対象となっており、一般的にはセキュリティや資金関連のトピックに触れるフラグがトリップしない限り、承認されます。
10%という割合は小さいですが、ここでの中心的な概念は機能しており、最終的には今後数ヶ月でその割合を拡大し、検証メカニズムをますます包括的に拡大できるかどうかにかかっています。これは最終的に、検証を改善できるかどうかにかかっています。これは、多くの技術(増分ロールアウトなど)が非常に効果的である高スケール複雑性、低ドメイン複雑性の製品よりも、低スケール複雑性、高ドメイン複雑性のソフトウェアで達成するには、より困難な種類の技術を必要とします。(誤解しないでください、それらは依然として機能しますが、ほとんどのユーザーが異なる設定と動作を持っている場合、それらが提供するカバレッジは少なくなります。)
人々はこの変更に一様に興奮しているわけではありません。これは、私たちが優れたエンジニアになるために学んできた方法からの大幅な変更であり、長年信じられてきたベストプラクティスが単なるドグマになったのではないかと再評価することは不快です。現在のサイクルの最も活気のある部分は、毎週これらのプラクティスを再評価できることです。最も痛みを伴う部分は、それらを再評価することをやめると遅れをとる可能性が高いということです。