コードレビューの話をいくつか見かけた. (1, 2, 3) 私もはやりにのってなにか書いてみたい. といってもリンク先についてどうこう言う気はない. ふだんからぼんやり感じていることをテキストにしてみたい.
コードレビューの様式
コードレビューのやりかたは色々ある. 話の背景をあきらかにすべく, まずは私が参加したり見聞きしたりしてきた方法を紹介したい. ただとりとめなく列挙しても見通しが悪いから, 方法を評価する軸を見立てておこう.
コードの粒度: 一回のレビューでレビュアが目を通すコードの量はどのくらいだろう. プロジェクト全体? モジュール単位, 機能単位, それともクラス単位? 古典的なレビュー様式はこれら <論理的な単位> でレビューをすることが多い. 最近はブランチやコミットのような <ひとまとまりの変更> を単位とする方法に人気がある.
Github の Pull Request はブランチ単位. 後者のコードレビューといえる.
タイミング: コードはいつレビューされるのか. <コミット前>, それとも<コミット後>? コミット後の<マージ前>? レポジトリはさておくと, レビューは毎日するの? 毎週? それとも不定期?
Pull Request はマージ前で頻度は不定期. だれか #reviewtuesday みたいのを流行らせて反応の鈍いレビュアをなんとかしてほしい.
人数: レビューを担当するの人は誰か. 同僚<一人>, チーム<全員>, それとも誰か<エライひと>?
形式度: レビューにはどんなルールやプロセスがあるのか. <チェックリスト>やテンプレ書類, <ツール>や静的解析はある?
Tavis CI は Github の Pull Request 相手にビルドを試せるらしい. これはレビュー支援用ツールと言えなくもない.
メディア: レビューはどこで行われるのか. 同僚の机や会議室, それともメール, Github? なにか専用の<ツール>はある?
こんな軸に並べると, 形式間の違いがわかりやすくなると思う.
準備はすんだ. 話を進めよう. 以下のリストは網羅的でもなく説明もおおざっぱだから, ここにない方法や詳しい手順を紹介したい人はどこかに書いて教えてください.
日次会議室レビュー
わたしが昔いた会社では, 開発チームに毎日 レビュー会議 があった. チーム(4-10 人くらい)が会議室に集まって, チェックインされたコミットをひとつひとつ調べていく. 司会は個々のリビジョンごとに意見をつのり, 合意ができたら次にすすむ.
コメントは議事録に残する. 該当リビジョンの作者はあとから議事録をみてコードを直す. レビュー内容を TODO リストとして管理するチームもあった. 反映漏れを避けるためだ.
評価軸に並べてみると: <チェックイン単位> で <コミット前> に <毎日>, <チーム全員> が <会議室> でレビューする方法.
手際の悪い方法だった. 他人がコードをながめている間の待ちぼうけは退屈だったし, 縁が薄いコードのレビューにも付き合わされる. 7-8 人の大きなチームにいた頃はいつも居眠りしていた. 当時 Twitter があったらずっとひやかしていたね.
進みが遅いせいでチェックイン速度にレビューが追いつかず, ときどきレビューをスキップして HEAD に追いつくデノミが発生した.
チームが 3 人くらいだと相対的なヒマさが下がるぶん効率よく感じた. それでもいま同じ事をしたら耐えられないと思う. 若者が書いたコードの設計などに色々説教して “レビューは教育的効果があるなククク” なんて悦に浸っていたのを思い出し赤面する. なんてうっとおしい… せめてレビューと説教は別枠でやればよかった. 本題に集中しないとね.
この方法が幅をきかせていたのはずっと昔, 何年も何年も前のこと. いまは違うと伝え聞く. 誰かに薦めたい方法ではない. 懺悔目的で紹介しておく.
ブランチレビュー
ともだちから伝え聞いた方法. 私はやったことがない.
そのチームは機能やバグ毎にブランチをする. そしてブランチを trunk へマージする前にレビューがあるのだそうな. Github の PR に似ている気もする. レビューはメールで依頼し, 指摘事項は専用のサイト(改造されたWiki?)にページをつくって記録する. 指摘事項をすべて反映するとレビューは受理されいざマージとなる.
<ブランチ単位> で <マージ前> に <随時>, <同僚一人> が <メールと Wiki> でレビューする方法.
教えてくれた人いわく “まあこんなもんじゃない?”. Poor man’s Pull Request ってとこか.
インスペクション風
あるとき書籍 ソフトウェアインスペクション を読んで感化された私の先輩が “これはすばらしい” と熱弁した. その話をもとに簡易バージョンのインスペクションを試したことがある:
レビューされるのはコード全体の一部. 開発者がチームにレビュー希望を申し出る. ややこしいコード, 新機能のためのまとまった量のコードなど, 書いたものに不安を感じたらレビューの出番だ.
レビュイはレビュー対象のファイルリストをチーム全員(当時は 4 人くらい)に渡す. もちまわりのレビュー当番は数日後にレビュー会議を設定する. チームは会議までにコードをにらみ, 問題点をテキストファイルやシートにまとめておく. 会議では各自が問題点を持ち寄って読み上げ, ひとつのリストにまとめる. 重複や意見の衝突はその場で議論する.
<機能(など)単位> で <コミット後> に <不定期> で <チーム全員> が <ファイルと会議> でレビューする方法.
日次会議室レビューよりは効率的. 各人は自分のページでコードを眺められるし, レビュー対象も限られているからだ. 一方で長続きもしなかった. レビュイの自己申告やレビュアの予習など, あまりに各人の善意や努力に依存しすぎていた. そのうえ自動化が弱く準備が面倒すぎた. ツールやプロセスで支援できればもうちょっとマシだったかもしれない.
本家 “ソフトウェアインスペクション” はもっと厳密でめんどくさい. 実現可能性はさておき, 定量的な品質に全力投球するレビューがどんなものかを考えるにはよい教材だとおもう. CMM とか TSP みたいのが好きな人むけ.
タスクカードと Review Board
コミット後のコードレビューを試したことがある. すでに trunk へコミットしたパッチを Review Board というツールでレビューする.
そのプロジェクトでは agile 風の開発を目指し 京大式カード を壁に貼ってタスクを管理していた. コードレビューをプロセスに組み込むため, “タスクを完了するにはコードレビューが必要” とルールを定めた.
開発者はコードを trunk にチェックインしたあと, その変更を Review Board にアップロードする. (Review Board にはアップロード作業を助けるツールがある.) するとレビュー用のページができるから, 開発者はチケットのカードにページ の ID を書いてレビュアに手渡す. レビュアは Review Board のサイト上でレビューする. 指摘をうけた開発者は修正をチェックインし, 差分を加えてパッチを更新する. 問題が片付いたらレビュアがカードにハンコを押して開発者に返す.
<機能(など)単位> で <コミット後> に <随時>, <同僚一人(以上)> が <ツール(Review Board)> でレビューする方法.
この方法はアナログのカジュアルさとツールの利便性がバランスしており気にいっていた. ただチームの小ささに依存する方法でもあった.
あと Review Board はコミット前レビュー向けにデザインされている機能が多く, コミット後レビューで運用するとちぐはぐな部分はあった. レポジトリに svn を使う手前チェックイン前レビューは難しかったので諦めていた.
コミット後レビュー
それほど一般的でないものの, コミット後レビューを使うプロジェクトは探せばある. たとえば Clang は trunk にコードがコミットされると メーリングリストに差分が流れる. レビューはその差分メールへの返信としてはじまる.
<コミット単位> で <コミット後> に <随時>, <気が向いた人> が <メーリングリスト> でレビューする.
ハードコアだと思っていたコンパイラのコードが案外カジュアルに開発されていてびびる. なおこれはコミッタに限った話. 新参物は従来通りメーリングリストにパッチを投稿して…とやる.
レビューツールたち
Review Board や Rietveld, Gerrit のようなウェブベースのコードレビューツールは, 特定のレビュー様式を想定している: <パッチ単位> で <コミット前> に <指名した一人(以上)> が <ウェブ上> でレビューする方法だ.
ツールは似たりよったりだけれど, “<指名した一人(以上)>” にはバリエーションがある. たとえば Chromium のソースツリーはディレクトリ毎に “オーナー” が決められており, チェックインにはオーナーのレビューが求められている. これはもともと検索会社由来の方法らしく, Chromium には去年導入された.
ツールにも細かい違いがある. たとえば Gerrit は Git と統合されているためパッチだけでなくブランチ単位でレビューできる.
開発フローはどれもだいたい一緒. 開発者はコードを書いてパッチやブランチをアップロードし, 所望するレビュアにレビューを頼む. 通知を受け取ったレビュアはサイト上に指摘を書き残す. レビュイは指摘を反映したパッチをアップロードしなおす. レビュアが良しというまでこれを繰り返し, 最後にコードをコミットする.
これらウェブ版レビューツールの先駆けは Python の親玉 Guido Van Rossum が検索会社向けに作った Mondrian だとおもう. これ以降, Github 以前の世代にうまれたウェブ企業は似たような仕組みを使っているところが多い, 気がする. たとえば Facebook は Phabricator という類似のツールをもっている. Twitter は Review Board を使っている もよう. 野次馬としては Phabricator が新しいぶん色々モダンでソーシャルにちがいないと勝手に期待している. PHP+MySQL が好きな他人は試して感想をおしらせください.
更に世代が進むと企業は Github に傾いているかんじ. Github はこうしたツールの進化形と捉えられているのだろう.
レビューツール以前
Mondrian や Github 世代の前は何が主流だったのか. 源流のひとつは伝統的なオープンソースの世界にみられる. たとえば Linux Kernel はいまだにメールベースでレビューをしているらしい. (ほんとに?) 検索会社も Mondrian 以前はメールを使っていたという.
もうちょっとだけ新しい世代には Bugzilla がいる. Bugzilla を使うプロジェクトでは, バグページにパッチをアップロードし, ページ内の掲示板風スレッドでレビューの議論を進める.
Bugzilla は Mozilla によって開発され, WebKit でも使われている. Bugzilla にはファイルを添付できる以上の大仰なレビュー支援機能はない. そのパッチがレビュー要求であること, パッチのレビューが受理されたことを, フラグとして管理できるだけ. でもフラグをはじめとするメタ情報を検索でフィルタできるから, メールよりはレビューの進捗を把握しやすい.
異形のものたち
メールや Bugzilla といった古いスタイルのコードレビューは, 古いソフトウェアと共に老いて消え行くだろう. けれど Linux Kernel や各種ブラウザのように元気なまま生きながらえてしまうことがある. すると古いプロセスはあやしげな改造を施され袋小路の異形に姿を変えていく.
たとえば git には git-am や git-send-mail など Github 世代のウェブっ子からすると使い道のわからない機能がある. おそらく Linux Kernel みたいなメールベースのコードレビューに使われるのだろう.
メールベースでも, こうしたツールのおかげで添付ファイルの扱いに煩わされることなくレビューに集中…できねえよ! アホか! と素直な反応が頭をよぎらないだろうか.
Bugzilla には Splinter と呼ばれる プラグインというか寄生スクリプトがある. Splinter をつかうと Bugzilla にアップロードされたパッチを ビジュアルに表示し, コードにインラインでコメントを追加, 表示できるようになる. まるで Rietveld のように … というかそれ以上 Bugzilla をいじめないで! どうかもうやめて! と魂の叫びが聞こえないか.
WebKit Bugzilla は謹製スクリプトを使い Splinter と似た仕組みをそなえる.
もうぜんぜん原型とどめてない…なお画面下にある緑のラベルはパッチがビルドに成功した印. 年寄りだって CI したい! のさ.
これら古参現役巨大プロジェクトは: <パッチやブランチ単位> で <コミット前> に <随時>, <指名されたレビュア> が <魔改造されたレガシーメディア> でレビューをしている.
古くて大きなプロジェクトは, 周辺のプロセスやツールが現行インフラの上に積み上がっている. そのしがらみに足を取られ, なかなか新しいシステムに移行できない. プロジェクト運営が民主的なほどツール乗り換えへの反対を無視しにくいせいでもある.
それでも SCM に限ると, Linux は BitKeeper から git へ, Mozilla は CVS から Mercurial に移行した. SCM は移行できてもコードレビューは移行できないのだなあ. レビューツールは単なるストレージである SCM と違い議論のためのメディアだから, そのぶんプロセスに露出している面積が大きいのだろう. でもがんばってなんとかしてほしいものです. いつの日か…
ペアプログラミング
話がそれた.
最後に紹介するレビュー様式はペアプログラミング. ペアプログラミングでは互いに一挙手一投足を見届けているから, 継続的にコードレビューをしているようなもの.
少し前, WIRED にこんな記事が載っていた: ”If Xerox PARC Invented the PC, Google Invented the Internet” 記事では検索会社のエースプログラマ Jeff Dean と Sanjay Ghemawat (GFS と MapReduce と BigTable を作った人たちだと思えばだいたいあってる) を取材している. なかでもあるエピソードが目を引く:
Even for Varda, who works on the team that oversees Google’s infrastructure, the two engineers are difficult to separate. “Jeff and Sanjay worked together to develop much of Google’s infrastructure and have always seemed basically joined at the hip,” says Varda. “It’s often hard to distinguish which of them really did what.
“All code changes at Google require peer review prior to submission, but in Jeff and Sanjay’s case, often one will send a large code review to the other, and the other will immediately ‘LGTM’ it, because they wrote the change together in the first place.” LGTM is Google-speak for “looks good to me.”
Varda means this quite literally. Over the years, Dean and Ghemawat made a habit of coding together while sitting at the same machine. Typically, Ghemawat does the typing. “He’s pickier about his spacing,” Dean says.
…
Google のインフラ統括チームで働く Varda から見ても, 二人のエンジニア (Jeff と Sanjay) を切り離して考えるのはむずかしい. “Jeff と Sanjay は一緒になって Google インフラの大部分を開発したのですが, 基本的にはいつも肩を並べていますね.” Varda はいう. “だから本当はどちらが何をしたのか, 大抵は区別のしようがないんです.”
“Google ではコードに加えるすべての変更に対し, 提出前のピアレビューをしなければなりません. ところが Jeff と Sanjay の場合, 一方がもう一方に巨大なコードレビューを送り, 送られた側が即座に LGTM するなんてことがしょっちゅうなのです. というのも二人はそもそも一緒にコードを書いているんですよね.” LGTM は Google 用語で “Looks good to me” を指す.
Varda がいうことはおよそ文字通りだ. 何年ものあいだ, Dean と Ghemawat は同じマシンに向かい, 一緒に座ってコードを書く習慣を培ってきた. ふだんは Ghemawat がキーボードを叩く. “彼の方が空白に細かいんですよ” そう Dean は言う.
検索会社員は全員ペアプロすべきじゃね？などと浅はかな考えが頭をよぎりました. そして本題に入りそこねたまま長くなりすぎたのでたぶんつづく.