- 開発者たちは GitHub の コードレビュー体験 に多くの不満を抱えており、それを改善するための新しい試みが行われている
git-review という実験的なツールは、コードレビューを ブラウザの Web インターフェース ではなく、ローカルでコードと一緒に直接扱えるよう設計されている
- レビューは 単一コミット で管理され、コード内に注釈のようにレビューコメントを残し、レビュアーと作成者がそのコミットを一緒に修正していく方式
- しかしレビューの途中でコードが修正されたりリベースされたりすると、競合処理 や
--force-with-lease の利用などで不便が生じ、大きな成功には至らなかった
- 結局は Web ベースのレビューに戻ったが、レビュー状態を Git リポジトリに直接含めるという発想は依然として魅力的であり、Gerrit-style Change-Id の導入など今後の Git 改善とともに、より良い代替案が登場する可能性がある
コードレビューシステムに対する問題意識
- 現在、多くの人が GitHub のコードレビュープロセス に不満を持っている
- 主な問題は スタックされたプルリクエスト や interdiff レビュー へのサポート不足に加え、
- レビュー状態がリポジトリ内部に保存されない
- リモート優先の Web インターフェース でのレビューが必須である
- 私が問題だと感じているのは、レビューの非中央集権性の不足 と インターフェースの非効率性 である
コード作成とレビューのワークフロー比較
- 人々はコードを書くとき、ローカルで エディタ を使う
- メモリや NVMe のレイテンシが少なく、ユーザー独自のワークフローに最適化された環境である
- コードレビューも ソースブランチをローカルに pull して作業する方式が好ましい
- Magit のようなツールを使えば、diff だけでなくコード全体のコンテキストも探索できる
- テスト実行、コード定義への移動、リファクタリングの試行など、強力な開発環境 を活用できる
- 一方で、PR に フィードバックを残すにはブラウザ で遅い Web インターフェースへ移動しなければならず、大きな diff では入力遅延もひどい
理想的なコードレビューインターフェースと保存構造
git-review のアイデアと実際の経験
git-review のアイデアは次のとおりである:
- コードレビューは PR ブランチ先頭の 単一コミット で行われる
- そのコミットに 特別なマーカー付きのコードコメント が追加される
- レビュアーと作成者がそのコミットを交互に修正しながら、push --force-with-lease に基づくコラボレーションを行う
- すべてのコメントが 解決済み表示 (
//? resolved) になり、レビュー終了時には revert コミット を追加して記録を残す
- アイデアは単純で実用的だが、実際には次のような問題が発生した
- レビュー中のコード修正 時に、下位コミットや新規コミットでコメントとの競合が頻発する
- force-push の過程で共同作業の摩擦が生じ、作業の複雑さが増す
- コードの変更履歴とレビュー進行の 不整合、およびマージ競合の管理が難しくなる
新しい変化と今後の可能性
- 今後、Git upstream に Gerrit スタイルの Change-Id が導入される可能性がある
- コミットごとの修正履歴追跡が容易になり、interdiff レビュー のサポート拡大が見込まれる
- ただし
git-review 方式とは一部で衝突が予想される
- 新しい Change-Id 構造では、コミット自体にレビューコメントを追加 するなどの新しいアプローチも可能になるかもしれない
結論と参考になるシステムの紹介
- 結局、現時点では Web インターフェースベースのコードレビュー に戻っている
- より良いソリューションの必要性は依然として残っている
- 参考になる関連システムやツール
- Fossil: すべての情報をリポジトリ内部に保持する SCM システム
- NoteDb: Gerrit のレビュー状態保存履歴を git に統合
- git-bug: issue 情報を git に保存
- git-appraise: レビュー情報を git 自体に保存
- prr: エディタ内で GitHub API と連携してレビューインターフェースを実装
- How Jane Street Does Code Review: より良い現実例の紹介
- git-pr: PR ワークフロー全体を git のネイティブ機能で置き換えるプロジェクト
まとめ
- まだ完璧な解決策はなく、より良い開発者体験 に向けた試みが続いている
- 今後の発展に大きな期待が寄せられている
2件のコメント
git-reviewのアプローチが良いかどうかは分かりませんが、GitHub ベースの PR レビューがひどいという点には同意します..Hacker Newsの意見
コードレビューで長年不満だったのは、本当に有用なフィードバック(些細な好みの指摘以外のもの)がほとんどいつも遅すぎること。レビューの唯一の(あるいはまれな)成果が「これは全部やり直して新しい設計で始めるべき」あるいは「そもそもこの作業は不要だった」という状況になってしまう。コードレビューこそが、すべての利害関係者が実際に参加して真剣に変更について考える唯一の時点のように見える。会議やJiraチケットで何か議論されていたのかもしれないが、組織内の別チーム・別部門の誰かがコードレビュー通知を受け取って初めてこの変更を知ることがよくある。自分も、別チームが妙なものを実装していたとき、コードレビュー通知で初めて知ることが多い。全員が事前にすべてフォローアップするのは現実的ではない。90年代の大学の授業では設計レビューもやっていたが、実務でそんなものを見たことはない。設計レビューがすべての問題を事前に拾える保証もないと思う。
SWエンジニアリングの世界には、実質的なエンジニアリングがあまりない。きちんとしたエンジニアリングプロセスの遅さを業界が受け入れられていない面もある。ほとんどのソフトウェアは致命的ではなく、バグやエラーはあとで直せる。橋、工場、航空機エンジンのように失敗が許されない他分野とは、賭けられているものやパッチの機会が違う。
うちのチームは4〜6人の小規模な開発者グループ。新機能を作るとき、頭の中で草案がまとまったらすぐ同僚に相談する。全員がそうしているので、コードレビューはコードスメルなど細かいことが中心で、全体アーキテクチャはたいてい2〜3人で事前に決める。チームメンバーがコードに同意しないと、お互い別のコードに手を入れたがらず状況が悪くなる。もっと規模が大きくても、責任をうまく共有すれば問題なくできると信じている。
コードレビューの時点でしかすべての利害関係者が参加しないのは、Gitやバージョン管理システムの問題ではなく組織の問題。PR作成の背景、チケットでの議論、意思決定の過程を共有できていないということ。これは機能不全の組織の例で、印刷された本が出る瞬間になって初めて全員が集まってきちんと関与するとして、出版プロセスを罵るようなものだ。
うちの組織では、根本的な設計判断については必ずRFCを書く。何が根本的な設計判断なのかは、その都度チーム内で自律的に判断する。Jiraのepic段階で詳細な実装方法が決まっていなければ、まずRFCを書くタスクを割り当てる。RFCはチーム内向けのこともあれば、ソフトウェアチーム全体向けのこともあり、後者の場合は2週間ごとに開かれる会議の前に全員が読んでコメントを残せる。大変でも、RFCベースの協調設計プロセスがない場所よりずっと良い。
自分の経験上、デザインレビューには共感する。以前はフォーマルな設計文書とレビューをしていたが、プロトタイピングと反復設計に切り替えた。設計段階では重要なディテールをしばしば見落としたし、すでに多くの時間を使っているので、あとになると雑に流してしまっていた。チーム全体で集まってレビューするのも非効率で、結局コードレビューの過程で問題が見つかる。設計文書を書くのが苦手だったり、やる気のない人も多い。結果として、5人を超えるとこうした非効率は避けられない。POと主要ユーザー、5人程度の開発者が一緒にいる環境が理想的だ。
HNでstacked pull requestについての投稿を見て、とても興味深かった。以前graphite.devを始めた頃は、FBやGoogleの経験者でなければこうした流れを知らないことが多かった。コードレビューのトレンドがこの3〜4年で驚くほど速く変わっていくのを見るのは面白い。
pre-mercurial arcanistの利用者として、いまだに大型PRやmerge commitなどで苦労しているチームにGraphiteを積極的に勧めている。特にPRとの連携を可能にした挑戦精神とその成果に強く感銘を受けている。Graphiteに、ハードコアな設定でrepoを初期化して、より強い前提を置けるprescriptiveモードがあればいいのにと思う。
Graphiteは素晴らしいソリューションなのに、価格がかなり高く、購買決定者を説得しづらい。Graphiteのような優れたツールがオープンソース化されるか、GitHubに組み込まれることを期待している。
fig workflowが少し恋しい。
最近CodeRabbitで起きたセキュリティ事故のせいで、LLMとコードベースが統合された新しいツールを試すのをためらっている。面白い新実験が、セキュリティ上の問題に変わりやすい。
stacked pull requestは本質的には不要な複雑さを追加する。頻繁で小さな単位の変更のほうが、はるかに良い開発習慣だ。trunk-based developmentやcontinuous integrationは、その目的によく合っている。
ますます多くの開発者が「レビューツールがどうあるべきか」について合意しつつある。いま重要なのは、それを本当に現実的に適用でき、持続可能なものにする組織やプレイヤーだ。最近のgit change-id導入は良い進展だと思う(jj、git butler、gerritなどに感謝)。GraphiteやGitHubは自分たちのユーザー向けの解決策に集中していて、みんなに開かれた方向ではない。多くのクライアントベースのコマンドラインツールも、それほど影響力がない。本当に必要な要素は以下の通り。
Githubに対する最大の不満は、アプリがあまりにも遅いこと。本当にブラウザタブが固まるほど遅い。Azure DevOpsは、これまで使ったコードレビューツールの中で最高だった。
Microsoft環境で.NET開発をするときAzure DevOpsを使ったことがあるが、本当に.NETエコシステムによく合うツールだ。
GitLabを本格的に使ったことがあるのか気になる。自分はbig 4の中ではGitLabがいちばん気に入っている。
DevOpsの何がそんなに良かったのか気になる。毎日使っているが、githubと似ているように感じる。むしろgithubの提案変更を自動反映する機能が恋しいこともある。
JavaScriptの大規模利用と高速リリース圧力が組み合わさって、こうした遅い環境が生まれている。それでもAtlassianよりはましだ。
gitをコードレビューに直接使うというアイデアは魅力的だ。変更点をローカルで直接触れるので便利。レビューを必ず単一コミットに合わせようとする理由がわからない。レビュアーが自分のコメントや修正をPRブランチに直接コミットするようなアプローチも面白い。これは伝統的なgithub flowと、Linuxのメーリングリストとパッチの流れのハイブリッドだ。
github PRが読み取り専用なのか気になる。チームメンバーが「suggestion」コメントでその場で修正案を出し、ボタンを1回押すだけでそのコミットに反映した経験がある。
レビューコミットが単一でなければならないというのは奇妙だ。複数人でレビューすれば同時編集の問題も起きるし、コードとも整合しない。レビューもそれぞれ別ブランチで作業し、最終的にsquash & rebaseする形が自然だ。議論が長引けば、comment commitも互いにチェーンとして残せる。重要なのは、このデータがmain branchの外のどこかに残ることだ。
コードレビューでは、ローカルブランチにpullしてsoft-resetし、まるで自分が書いたもののように見るのを好む。コミットがうまく分割されていないと、共同作業者がそれぞれ分け直してレビューしなければならず非効率だ。レビュー対象が膨大だと、誰かが完全に理解したと言うのは難しい。全体は単に部分の総和ではない。
簡単なコードレビュー用シェル関数の例を共有する。clean treeの状態でreviewブランチをプリセットでcheckoutし、nvimでdiffを確認し、作業後はブランチ整理まで自動化する。
良いコミットと、それを元にしたPRをうまく作ることも、コードをうまく書くのと同じくらい重要なスキルだ。しかし実際には、PRの分割やコミットメッセージ作成が苦手な開発者が思った以上に多い。
データサイエンスの並列作業チームでは、コードレビューのために2〜3本のブランチをcheckoutして使っている。
PRレビュー用に https://github.com/sindrets/diffview.nvim をNeovimで使っている。vscodeのdiffに似たUIでありながら、vimのdiffモードを活用している。軽いレビューには
git log -p --function-contextも便利だ。1人が最初のdraftを書き、別の人がそれを整えてmergeする方式に関心がある。実際にやったことがある人はいるだろうか。
実際のコーディングの90%以上は1人が担当し、レビュアーが直接最終反映とmergeの責任を持つスタイルに賛成だ。前職では、レビュアーが必ずmergeを担当し、大きな変更ならコメントだけを伝えていた。誰かにいちいち承認クリックを頼む方式より、こちらのほうがずっと効率的だ。
レビューコードの作成に関わると、はるかに深いフィードバックができると感じる。こうした構造は、「自分のコード/あなたのコード」ではなく「私たちのコード」として認識させてくれる。TDDのような反復的・協調的な文化によく合う。
この方式は非同期ペアプログラミングに似ている。
trunk-based developmentでペアプログラミングを活用する人たちを知っている。2人でコードを書き、OKならそのままmainにmergeし、テストが通ればすぐデプロイする。実際にうまく回っている。
VSCodeでGithub Pull Request拡張プラグインを使ってローカルレビューしているが、かなり便利だ。エディタ内でそのままコメントを付けてレビューできる。
Github Webでは変更ファイルが多いとファイルを隠すが、VSCodeでは自由にナビゲーションできてずっと快適だ。この機能がVSCode / Githubの組み合わせで実現されている以上、他のエディタにも広がる可能性はあると思う。
改善しているのは確かだが、それでもGithubとVSCodeの双方向vendor lock-inは深い。
Github WebでPRを開いて「.」キーを押し、そのままVSCode Webでレビューするのも、はるかに良い体験だ。
Gitにfirst classなchange IDが入るという話は本当にうれしい。FacebookのPhabricator diffs revision trackingに似ている。もっと詳しく調べられるリンクが知りたい。
Sourcehutにも触れておきたい。クラシックなパッチ/issue/bug/議論をメールでやり取りする流れを受け継いでおり、メーリングリストやCIと連携している。Drew Devaultの git-send-email.io、git-am.io では、パッチ送受信方法のリソースも提供している。