« TiDDを実践して気づいたことpart8~事象にユニークなIDを採番するとコミュニケーションが促進される | トップページ | 【公開】SEA関西プロセス分科会講演資料「TestLinkのベストプラクティス~日本の品質管理技術を見直そう」 »

2011/01/15

ReviewBoardでプレコミットレビュー

岡本さんがReviewBoardの素晴らしい記事を掲載されていたのでメモ。

【元ネタ】
Review Board | Take the pain out of code review

Review Boardならコードレビューを効率良くできる! (1/3) - @IT

InfoQ: Review Board - コードレビューをオンラインで

ReviewBoard 釣られて使ってみた | tsuyuki.makoto

Rietveldでレビューしよう @ val it: α → α = fun

徒然さめざめ Redmine Hack! reviewboardとの連携

ReviewBoardのインストール方法: プログラマの思索

ReviewBoard 釣られて使ってみた | tsuyuki.makotoには下記のように説明されている。

(前略)
Tracのような感じなのかと思っていたけれど、実は全然違うものでした。
ReviewBoardは、『改変したコードを、Subversionにコミットする前に、レビューを受ける』ためのツールです。
(後略)

つまり、ReviewBoardはSVNコミット前にコードレビューを実施するプレコミットレビュー(pre-commit review )が基本的なワークフローだ。

Review Board | Documentation | General Workflow

InfoQ: Review Board - コードレビューをオンラインででは、下記のようなワークフローになる。

1. ユーザーが何かとてもクールな変更を思いつき、ローカルへチェックアウトしたコードを変更する
2. ユーザーがレビュー依頼画面で差分を登録し、説明を書き、何人かのレビュアーを選ぶ.
3. ユーザーはそのレビュー依頼の「公開」ボタンを押して、レビュアーの誰かが見てくれるのを待つ
4. 誰かがレビュー依頼を見てこうコメントする。「なんてクールなんだ。ただちょっとおかしいな」
5. ユーザーはレビュアーがコメントした点を直すためにコードを更新する .
6. ユーザーは更新した差分を登録し、レビュアーのコメントに何を変更したか(あるいはなぜ提案されたような変更をしなかったか)返答する
7. レビュアーが更新されたコードを見てゴーサインを出す
8. ユーザーは変更をレポジトリにコミットする
9. ユーザーはさっきのレビュー依頼画面の「承認済み」ボタンを押して、レビュアーのダッシュボードからリクエストを消す

プレコミットレビューの利点は2つある。
一つは、レビューを完了していないソースはコミット不可にすることによって、SVNリポジトリの品質を保持出来ること。
XPならば、テスト駆動開発によって単体テスト済みの品質を保障するが、プレコミットレビューを使えば、二人の目による品質チェックを更に補強できる。
コードレビューをやっていないチームでは、コミットした後に間違いを発見して更に修正するといった2度手間が発生して、デグレードの危険が高くなる。

もう一つは、コードレビューの作業履歴がWeb上で一括管理できるので、レビューアとレビューイのコミュニケーションのやり取りが促進されること。
Redmineによるチケット駆動開発、TestLinkによるテスト管理も同様だが、複数人の共同作業をWeb上で一括管理できれば、作業履歴を検索するのも簡単なので、インタラクティブなコミュニケーションが可能になる。

従来は、レビューの作業履歴をExcelやメールで残していたりする。
Excelの欠点は、ドキュメント駆動になりがちで、指摘事項を探すのが面倒だし、指摘が反映されたかどうかチェックするのも面倒なこと。
メールの欠点は、返信メールに履歴が残ることによって、メールそのものが長くなりがちで、レビューの経緯を追跡しにくいこと。

ReviewBoardの使い道としては、岡本さんの記事に書かれているように、大規模プロジェクトでレビューの重要性が非常に高い場面で使うと効果的だと思う。

但し、ReviewBoardはソースのDiffを自分で作って、そのパッチをアップロードする手間がかかるために、運用がちょっと面倒な時がある。
最近は、MercurialやGitのような分散バージョン管理があるのだから、レビュー前のコードラインとtrunkを別々にブランチ管理しておき、レビュー後にtrunkへPush又はRebaceするワークフローの方が良い気がしている。

ReviewBoardの人達もそのワークフローは意識しているようで、分散バージョン管理と組み合わせたpost-commit reviewのツールも作っているようだ。

Review Board | Documentation | post-review

僕は試していないのでよく分からないが、上記の記事によれば、RBToolsというツールを使えばGitやMercurial、Perforce、SVNなどでpost-commit reviewが可能らしい。

色々試してみたいと思う。

|

« TiDDを実践して気づいたことpart8~事象にユニークなIDを採番するとコミュニケーションが促進される | トップページ | 【公開】SEA関西プロセス分科会講演資料「TestLinkのベストプラクティス~日本の品質管理技術を見直そう」 »

Agile」カテゴリの記事

Git・構成管理」カテゴリの記事

Mercurial」カテゴリの記事

ソフトウェア工学」カテゴリの記事

コメント

Mercurial を使用する場合、MQ のパッチとして修正を実施すれば、"hg qrefresh" での変更取り込み後に .hg/patches 配下のファイルを upload することで、比較的簡単に差分ファイルを作成できますね。

以下、(個人的に)修正単位のブランチ管理が不向きと思われるケースです。

例えば、修正/機能追加の採択決定権が無い場合(e.g.: 受託開発のプロダクト)、必ずしも全ての変更が採用されるとは限りません。

そのような場合、修正/機能追加毎にブランチを作成すると、最悪「長期間(あるいは永遠に)マージされないブランチ」が多数残留するような状況が考えられます。

そうなると、ベースソースの更新などにより(「リベースが困難」など)賞味期限切れなブランチも出て来ますし、そもそも未 close なブランチが多数ある状況は、保守上あまりよろしくありません。

そういった場合には、パッチによるプレコミットレビューでの運用の方が向いていると思います。

# 私自身が受託開発案件で上記のような運用をしていました

但し、同一修正に対してパッチが複数版存在する場合、会話が噛み合わなくなる可能性もあるため、パッチバージョンを識別するための情報を入れる等の工夫が必要かもしれませんね。

投稿: フジワラ | 2011/01/15 19:40

◆フジワラさん

コメントありがとうございます。
なるほど、トピックブランチをたくさん作る弱点もあるわけですね。
post-commit-reviewの方が優れていると思っていましたが、pre-commit-reviewの方が良い利点もある指摘は参考になりました。

投稿: あきぴー | 2011/01/15 22:17

この記事へのコメントは終了しました。

トラックバック

この記事のトラックバックURL:
http://app.cocolog-nifty.com/t/trackback/49479/50590019

この記事へのトラックバック一覧です: ReviewBoardでプレコミットレビュー:

« TiDDを実践して気づいたことpart8~事象にユニークなIDを採番するとコミュニケーションが促進される | トップページ | 【公開】SEA関西プロセス分科会講演資料「TestLinkのベストプラクティス~日本の品質管理技術を見直そう」 »