非同期コラボレーション手段としてのコードレビュー

CARTA HOLDINGS アドベントカレンダー 2023 20日目の記事です。

Lighthouse Studio の海老原 (@co3k) です。あけましておめでとうございます。あけましておめでとうって時期でももうないか。。。えっと、とにかく何かしらおめでとうございます！

さて、コードレビューの話です（が、考え方のエッセンスはコード以外のレビューにも適用できるはずです）。別に示し合わせたとかではないのですが、 10 日目を担当してくれた ryu さんが既にレビューに関する記事を書いてくれています。

即返すプラクティスは僕も実践しています。「もうレビューされたw」みたいなお声をいただけたりして、とても嬉しいです。（誰ですか「レビューは即返すのにアドベントカレンダーは即書かないんですね」なんて言っているのは）

ご多分に漏れず弊チームでもレビュー待ちによって生じるリードタイムの長期化は課題として上がっており、チームの方針として「朝会でレビュー待ち状態の pull request がないかを確認する」「レビュワーになった人は午前中のうちにレビューする」を定めつつ、特定の人にレビュー負担が偏らないように週次でレビュー担当数を可視化しておく、という運用をしています。

そうはいってもユーザに即座に価値を届けたいじゃないですか。という欲求があまりにも強すぎるので、このフローに先んじて「即返す」を発動してしまいがちです。圧倒的スピード大事ですからね。

そもそも……我々は何のためにコードレビューをしているのか？

アジャイル開発が一般化し、小さくて頻繁なリリースが求められるようになりました。それに呼応するようにバグトラッキングシステムやバージョン管理システム等の各種周辺ツールも進化を続け、チケットやユーザーストーリーなどの単位でのコードレビューが自然な形でおこなえるようになってきました。

また、 OSS 開発の文脈においては、「（時として顔も知らない）コントリビュータから受け取ったパッチを自分たちのソフトウェアの一部として取り込むかどうか」という観点でのレビューが必要とされていたため、 GitHub などのバージョン管理システムのフロントエンドではコードレビューのための機能等を備えているのが一般的でした。これらのシステムが徐々に OSS 開発に限らない領域まで広がり、ツールとしても文化としても合流した結果、コードレビューというプラクティスがいまではほとんど当たり前と言える形で浸透しているように感じます。

一方で、最近では、先述したようなコードレビューの引き起こすリードタイム等への影響などに対する怨嗟の声もぽつぽつと聞こえてくるようになりました。

ここで、「当たり前だから当たり前なのだ」で終わらせないのが我々 CARTA HOLDINGS のエンジニアの「本質志向」です。このタイミングであらためて、コードレビューの意義について再確認していきましょう。……と言いたいところなのですが、ありがたいことに、『Software Engineering at Google』 (2020 年) の 9 章 "Code Review" 内、 "Code Review Benefits" にて概ね以下のようにまとめられています。

コードが正しいことの確認 (Checks code correctness)
他のエンジニアにとって理解が容易な変更であることの担保 (Ensures the code change is comprehensible to other engineers)
コードベースにおける一貫性を強制 (Enforces consistency across the codebase)
チーム内のオーナーシップの心理的促進 (Psychologically promotes team ownership)
知識共有 (Enables knowledge sharing)
コードレビュー自体の歴史的記録の提供 (Provides a historical record of the code review itself)
- コードを調査する際に歴史を辿っていくことがあると思いますが、そうした際にレビューの記録がインサイトとなるよ、ということですね

これらのベネフィットを享受しうるということには、皆さんも同意されるところなのではないでしょうか。僕も同意します。

個人的には、ここにひとつ、 「内部不正の防止」 を付け加えたいです。いわゆる Dev と Ops を兼任する我々フルサイクルエンジニアはソフトウェアに対する絶大な権限を有しており、やろうと思えばどこまでも凶悪なことができてしまいます。安定した事業運用のうえでは、こうした脅威に対して自らで制約を課す必要があり、その制約の一つとしてコードレビューにおける相互確認が有効であろうと考えられます。

For us なコードレビューについて考える

ということで、 "Code Review Benefits" に「内部不正の防止」を付け加えた 7 項目について検討していきます。ここでは一例として弊チームにおける "for us" なコードレビューについて考えてみます。みなさんもよければ真似してみてください。

For us な点

弊チームにとっても嬉しそうな恩恵としては以下があげられそうです。

チーム内のオーナーシップの心理的促進
知識共有
コードレビュー自体の歴史的記録の提供
内部不正の防止

オーナーシップを持ち続けるために

プロダクトに対するオーナーシップを持つフルサイクルエンジニアである以上、コードに対するオーナーシップを全員が持っていることはもはや当然の前提です。コードを書いた人だけではなく、レビューすることによってオーナーシップを持つ人が広がるのであれば、まさに一粒で二度おいしいと言えそうです。

また、我々はプロダクト内の各マイクロサービスにおいてあえて固定の担当者を設けずに、全員が全体を見るスタイルでやっています。つまり、「いま自分が書いているコードは他の誰かがメンテするかもしれない」可能性が、「いつか」ではなく「常に」あるのです。そのため、同期的な知識共有の機会はもちろん、非同期的な歴史的記録の提供も言わずもがな重要です。コードレビューにおいておこなわれた疑問点の解消や議論が、将来コードをメンテする他の人々にとって有益となることは間違いありません。

オーナーシップを持つからこそ

また、先述したように DevOps を兼任する以上、内部不正への抑止力としての効果も見過ごせません。弊チームにおいては開発者がプロダクション環境に対して直接的にデプロイすることはできず、必ず CI / CD を経由する必要があります。こうした前提があるため、プロダクション環境で不正なコードをデプロイするとしたら、通常の pull request に分からないように混ぜ込むというのがもっとも現実的な手段であると言えます。この脆弱性に対して、コードレビューというパッチを当ててリスクを軽減するというわけです。もちろん手の込んだ不正を防げるような完璧なものではありませんが、こうした相互確認のステップを設け、「不正のトライアングル」における「機会」を提供しないことで、心理的なハードルを上げることは内部不正対策として有効であることが知られています。

Not for us な点

一方、我々にとってはあまり嬉しくなさそうな恩恵としては以下があげられそうです。これは、もう少し正確に言えば、「もうすでに他の要因によって解決可能な課題が解消していたり、リスクヘッジができている」ので、「コードレビューでの解決を期待しなくてもよい」というようなニュアンスであるとするほうが実情に近そうです。

コードが正しいことの確認
他のエンジニアにとって理解が容易な変更であることの担保
コードベースにおける一貫性を強制 (ちょっと嬉しい)

コードが正しいことの確認に関していえば、原初よりコードレビューの目的として理解されているところだと思います。『CODE COMPLETE 2nd Edition』（2004 年）において "Code Reading" として紹介されている (Chapter 21. Collaborative Construction) ものの亜種が現在の我々にとっての「コードレビュー」に相当するものと思われますが、ここでは also comment on qualitative aspects of the code としながらも、「不具合検出率」がテスト等と比べて 20% から 60% ほど優位であるとするなど、明らかにバグの発見を主目的であると捉えていることがわかります。

現代的なウェブサービスの自社開発における「コードの正しさ」を目的としたレビューの意義

ただし反復的かつ短いイテレーションのなかで継続的にデリバリ、検証、改善のサイクルを回し続けていくアジャイル開発のうえでは、このメリットは以前と比べてグッと小さくなりつつあると言えるでしょう。ましてや我々の場合はウェブサービスを自社開発しているわけで、顧客に価値を届けるためのリードタイムは限りなく小さく抑えられます。また、ブルーグリーンデプロイをしているため切り戻しも容易かつ即座におこなえます。であれば少ない人数で時間を掛けてバグを精査するよりも、致命的な問題さえなければ即座にデリバリーしてしまい、問題があれば改善するもしくは切り戻しするのが合理的です。まさに Given enough eyeballs, all bugs are shallow (充分な目玉の数があればどんなバグも洗い出せる; リーナスの法則) というわけです。

コードレビューにおける「自転車置き場の議論」

こうなってくると、コードレビューの意義というのはバグのような外形的に発露しうるものを発見するというよりもむしろ、いわゆる内部品質というところに重きを置かれつつあることがあります。内部品質の向上——自転車置き場の議論を誘発しかねない甘美かつ危険な香りがしますね。

危険なので対策を考えたいところです。自転車置き場の議論を避けるうえで有効な手立てとして、意思決定者を設ける、というアプローチがあります。個人の好みや意見に左右されない絶対的な存在というわけですね。

静的解析技術が発展し広まった現代的なプログラミング環境においては、こうした絶対的な存在としてフォーマッタや lint ツールが据えられがちです。我々とて例外ではなく、 gofmt のようなスタンダードなフォーマットツールがある Go などを好んで使うことによってこうした問題を最小限に抑えています。また、好むと好まざるとに関わらず、可能な限り言語の標準的な書き方に従うこと、既存のコードにあわせること（それが難しいようなら局所最適に終始するのではなく全体を変えること）を求めています。

コードの書き方の問題について人間の好みが介入する余地を減らせるとして、終わりない議論を生じさせる他のトピックといえばアーキテクチャがありそうです。とはいえこれは当該リポジトリの first author が design doc 等で定義したものを起点とすることでそうズレなくなるのではないでしょうか。また、特に、我々がおこなっているコードレビューはあくまで pull request ベースでのコードレビューであって、コードセット全体に対するものではありません。このレベルの議論をコードレビューという狭い場でおこなうことは、まさに木を見て森を見ずです。弊チームでは、こうした議論の機会を週次で設けており（議題がない場合は開催せず）、コードレビューでこみ入った議論をすることを、むしろ極力回避しています。

非同期コラボレーション手段としてのコードレビュー

タイトル回収のある作品は名作であると風の噂で聞いたため、そろそろこのあたりでタイトル回収をしていきます。

さて、ここまで考えてみると、むしろ我々がコードレビューに期待していることは、フォーマットこそ原初からのコードレビューを踏襲しているものの、求める役割はそれとはまったくかけ離れていることがわかります。

先ほど言及した『CODE COMPLETE 2nd Edition』にも見られたように、コードレビューは監査やテストなどの工程を補完ないし代替しうるものとして考えられてきました。これは言うなれば第三者的な視点の獲得です。

一方で、我々が期待するコードレビューの役割は、第三者というよりむしろ当事者を増やすことを目的としていると言えます。これは先述したような監査やテストというよりむしろペアプログラミングなどのコラボレーション行為との類似性が強いように思います。ペアプログラミングが同期的なコラボレーションとするならば、我々のコードレビューは非同期的なコラボレーションであると位置づけることができるかもしれません。

そこで、あらためてペアプログラミングの利点について整理してみましょう。

主眼ではないながらも、品質向上の効果があります。既に触れたとおり、『CODE COMPLETE 2nd Edition』において「不具合検出率」の優位性が確認されています
生産性向上の効果が知られています。この理由として、『Extreme Programming Installed』では「飽きにくい (work longer without getting tired)」、思考作業の分担によって「頭の中で生じるスワップ (mental swap)」が生じなくなること、『Joel on Software』の「射撃しつつ前進 (Fire And Motion)」では、簡単なように思えてとても難しい「ただはじめること (just getting started)」ことが生産性における鍵だとしたうえで、「ペアが互いに『はじめること』を強いる (you force each other to get started)」ためにペアプログラミングはうまく機能するのだ、としています
知識共有 (コードベースはもちろん、考え方、プロセス、ツールの使い方など) の機会となります。自分とは異なる人の手を通じて、自分とは違う道を進みうるその過程をリアルタイムで体験できるわけです。知的好奇心が刺激されることは間違いないでしょう
言わずもがな、コミュニケーション、チームワークの向上に繋がります。同じ問題に一緒に取り組んだバディとの間に信頼関係が築かれることは想像に難くありません

こうした利点を非同期的なコラボレーションとしてのコードレビューにおいても享受するにはどうすればいいでしょう。同期性を求められる取り組みと異なり、非同期的な取り組みには時間の都合を合わせる必要がないというメリットもありますが、フィードバックが即座におこなえないというデメリットもあります。

特に、ツールの使い方などのコーディングプロセスに関わる知識共有などは強い同期性を要求します。しかし、さほどの同期性を要求しない要素であれば、いくつかの工夫によって非同期の元でもメリットが享受できるかもしれません。

生煮え段階のコードレビューおよびマージを許容する

原初からのコードレビューのコンセプトに従えば、コードレビューの提出はあるひとまとまりが完成した「区切り」のタイミングで提出したくなるかもしれません。私たちはそこに期待しないと決めたわけですから、必ずしもコードレビューのタイミングは完成時に限定しなくてもいいわけです。

GitHub には draft pull request もありますし、途中だけどこんな感じでいいかちょっと見てみてーって相談しつつ、並行して作業を進められるのはいいですね。まあ全然 draft とかに頼らなくても普通に pull request 出してわーってコメントもらって、いったん仕切り直しますわーって言ってもう一回 pull request 出し直してとかでも全然いいわけですが。

また、海老原自身は完成系を待たずにマージすることも推奨、実践しています。これはたとえば、いわゆるステルスリリース（「既存機能の導線を断ったうえでリリースする」、「特定のクエリパラメータを付与しないと機能が発現しないようにする」など）であったり、 MVP (Minimum Viable Product) の段階に達した時点でリリースしてしまって、あとは走りながら改善する、といったプラクティスを、レビュー目的においてもおこなうということです。もちろん、他の機能に影響を与えないとか、ユーザに混乱を与えないとか、ブランドイメージを損なわないとか、諸々の条件が整ったうえでということになりますが、フィーチャーブランチの機能を手元で動かすように準備する手間を掛けることなく、ふたりが一緒の環境であーだこーだ言えるのでこれはめちゃくちゃアリな感じあります。

もはや死語かもしれませんが、 Web 2.0 黎明期ではよく「永遠のβ版」という表現が使われていました。これは、サービスは「完成」することなく、継続的に改善、進化し続けるということをセンセーショナルに謳った表現です。リードタイムが限りなく抑えられ、トライアンドエラーがしやすい Web というプラットフォーム特性を的確に捉えた表現であるように思います。開発環境が整備され強化された現在において、あらためてこのコンセプトに立ち返り、「Web 2.0 をちゃんとやる」、つまり、「Web 2.0 2.0」をやっていく、というのも乙なものかもしれません。

「即返す」プラクティスを実践する

ペアプロといえばやはり同期的な対話による迅速なフィードバックが魅力です。非同期的なアプローチでも少しでもこの恩恵を受けるためにはやはり「即返す」しかありません。しかし愚直に「即返す」だけではレビュワー側のコスト爆上がりですし、中途半端なことをせずに素直にペアプロをしたほうがいいまであります。

「即返す」を無理なく実践しやすくするためには、月並みですが、タスクの細分化をめっちゃ頑張るとか、フェーズ分けをするなどして、「pull request のサイズを小さくする」というのが効きます。

ここで問題となってくるのは、「pull request のサイズが小さければ小さいほどめっちゃコメントしたくなってしまう」という謎の衝動でして、どうやらこれは万国共通のようです。

10 lines of code = 10 issues.

500 lines of code = "looks fine."

Code reviews.
— I Am Devloper (@iamdevloper) 2013年11月5日

「即返す」をチームとして実践するためにも、レビュワー側ではこうした重箱の隅をつつきたくなる誘惑に打ち勝つ自制心が必要になります。頑張ってあらがいましょう。僕も頑張ってあらがいます。

あえて特定の担当者をつけないようにする

特定の担当者をつけずにプルリクエストをレビューすることで、全員がコードベース全体に対してオーナーシップを持つことができます。これは全員がコードの品質保持に貢献する体制を作るうえで非常に有効です。

この取り組みは Lighthouse Studio 開発チームのスタンダードなスタイルとして従来より採り入れていましたが、そうはいっても、「この領域はこの人が詳しいから」が積み重なって、気がつけば特定の人にレビューが集中してしまいやすくなるような力学がどうしても働いてしまっていました。これは組織や事業規模の拡大に伴う、プロダクトバリエーションの増大も背景にありますし、「監査」としてのレビューへの期待がどうしても捨てきれていないことへの証左とも言えます。

我々としては、先述したような「レビュー担当数の可視化」に加えて、特定の人に知識が偏っていそうなプロダクトについては、一定期間で交代する補助的な「壁打ち相手」を設けるようにし、少しずつチーム全体に知識を伝搬していくような施策をはじめました。このあたりも今後どうなったか触れていけるといいですね。

すべてのレビューコメントは暗黙 nits あるいは暗黙 imo

レビューコメントに [nits] とかのラベリングをつけていくようなスタイルも最近ではごくごく一般的となってきました。これは極めて合理的というか、フレームを避けたりチーム内でのいい感じのコミュニケーションを築いていくいい施策であることは確かです。

しかしここにはひとつ落とし穴があって、裏を返すと「何もついていない」コメント、つまりフラットな状態のコメントの強制感を相対的に暗示してしまう、ということでもあります。

私たちはレビューに監査的な役割を期待しないのですから、よっぽどのことがない限り全部 nits であり imo をつけていくべきです。……というのはなかなかまどろっこしいですね。なのでデフォルトで [nits] もしくは [imo] がついている、つまり暗黙 nits あるいは暗黙 imo なわけです。

レビューコメントでは別にレビューしなくていい

レビューに監査的な役割を期待しないということは、レビューも別に指摘とかだけじゃなくていいってことですね。「え、この書き方オシャレくない？」とか、「このバグどうやって見つけました？」とか、ガンガンコミュニケーション取っていっていいわけです。非同期ながらもペアプログラミングのようなコードを通した会話ができるのが、 pull request 等のコメント機能の最大の魅力であると思っています。

しかもペアプログラミングとは違って、しっかり記録に残るのが非同期コミュニケーションの最大の利点です。何気ない会話が、しかも発言した当人としては大したことだと思っていなかったようなものが、時を越えていつかチームの誰かの役に立ったりすることもままあります。