OSS開発、“はじめの一歩”で抑えておくべきポイントオープンソースソフトウェアの育て方(4/5 ページ)

» 2009年08月19日 14時05分 公開
[Karl Fogel, ]

きちんとしたコードレビューの習慣

 開発コミュニティーをうまく育てていくために最適な方法は、開発者同士がお互いのコードを見せ合うことです。これを効率的に行うには、技術的な支援が効果的です。特に便利なのが、コミット時のメール通知です。これについてはコミットメール項で詳しく説明します。コミットメールとは、誰かがソースコードに加えた変更をコミットするたびにそのログメッセージと変更内容をメールで送る機能のことです(バージョン管理に関する用語集項差分を参照してください)。コードレビューとは、コミットメールがやってくるたびにその内容を確認し、バグや改善点がないかどうかを探す作業を指します*13

 コードレビューは、さまざまな点で役立ちます。オープンソースの世界におけるコードレビューの最大の効果は、ソフトウェアの品質を一定に保つことです。ソフトウェアにバグが含まれる原因は、コミットされたコードをきちんとチェックしなかったことにあります。コミットの内容を多くの目でチェックすることで、バグを残したまま公開してしまう危険性を減らすことになります。しかし、コードレビューの目的はこのような直接的なものだけではありません。コードレビューによって、自分たちの作業の内容を確認させることができます。コミットの内容をレビューするには、その作業についてよく知っておく必要があるからです。ほかの人が自分の作業を評価すると知っていれば、人は最善を尽くして作業を行うようになります。

 レビューは公開の場で行わなければなりません。開発者同士が物理的に同じ部屋にいる場合でも、誰かがコミットしたときにそのレビューを部屋の中だけで済ませてしまってはいけません。面倒でも開発者用のメーリングリストに投稿しましょう。そのレビューを見ることで、参加者全員が何かを得ることになります。レビューの結果、何らかのコメントをしたり、場合によっては不具合を見つけたりすることもあります。レビューは日常の作業の1つと捉えましょう。そう。ちょうど皿洗いや芝生の手入れと同じような感覚です。

 Subversionプロジェクトの開始当初は、日常的なコードレビューの習慣はありませんでした。すべてのコミットがレビューの対象になる保証はありませんでしたが、変更されたコードの内容に興味のある人がそれを眺めることはありました。当時のコードに紛れ込んでいたバグは発見可能でしたし見つけられるべきでした。当時の開発者の1人であるグレッグ・ステインは、過去の経験からコードレビューが役立つことを知っていました。

 彼は、リポジトリへのすべてのコミットの1行1行の内容を詳細にまでわたってレビューするようになりました。誰かがコミットするたびにそのコミットについてのグレッグからのメールが開発者用メーリングリストに流れました。メールの内容は、コミットの内容を細かく切り刻んで分析し、起こりうる問題を指摘するといったものです。時には、優れたコードに対して賞賛を送ることもありました。コミットがあるたびに、バグを見つけたり注意しないとつまづくく恐れのあるあまりよい書き方ではないコードを見つけたりしていたのです。彼は、コミットの内容をレビューするのが自分1人であることについて、表立っては不平を述べることはありませんでした。かなりの時間をレビューに費やしていたにもかかわらずです。その代わりに、ことあるごとにコードレビューの素晴らしさについて発言していました。

 やがて、わたしを含めたほかのメンバーもコミットの内容を定期的にレビューするようになりました。何がそうさせたのでしょう? 決してグレッグがわたしたちにそれを強制したわけではありません。彼は「コードのレビューが時間を掛けるだけの価値のあるものである」こと、そして「他人のコードをレビューすることは新しいコードを書くのと同じくらいに重要である」ことを自身の行動をもって証明したのです。いざそういう習慣が身につくと、自分のコミットに何の反応もなければ逆に不安になります。開発者用メーリングリストで「誰かわたしのコードをレビューしてくれないかな?」と聞いてみたりなんかして。後に、グレッグは別の仕事を得ることになり、Subversionにあまり時間を掛けられなくなりました。彼は定期的なレビューをあきらめざるを得なくなったのです。しかし、そのときにはすでにレビューの習慣がわたしたちにも深く根づいていました。まるで太古の昔からそれが当たり前であったかのように。

 コードのレビューは、最初のコミットの時点から始めましょう。差分を見れば簡単に発見できる問題としては次のようなものがあります。例えばセキュリティ上の脆弱性やメモリリーク、コメントやAPIドキュメントの不足、「1つずれちゃった(off-by-one)エラー」、呼び出し元と呼び出し先の規約の不一致などです。また差分の前後を確認することで発見できる問題もあります。しかし、より規模の大きい問題、例えば繰り返し登場するパターンを一カ所にまとめていないことなどは、定期的なレビューをしていないと見つけにくいものです。過去の更新の履歴の記憶を今回の差分と組み合わせることで、このようなの問題も見つけやすくなるのです。

 何もコメントすべき点が見つからないんじゃないかとか、すべてのコードについて熟知しているわけじゃないとかいったことを気にしないでください。あらゆるコミットには何かしら言うべきことがあるはずです。何も疑問点が見つからなかった場合は、何か賞賛の言葉を贈ればいいだけのことです。大事なのは、すべてのコミッターに対して「あなたの作業内容を見ているし、理解もしている」というメッセージを伝えることです。もちろん「後でほかの人にレビューしてもらえるから、コミット前のチェックやテストは不要」というわけではありません。本来自分自身で発見すべき問題を、他人のレビューに頼ってはいけません。

[2] これは、オープンソースプロジェクトでごく一般的に行われているコードレビューの方法を示したものです。より中央集権的なプロジェクトでは、複数の人が集まってソースコードのプリントアウトを見つめ、特定の問題やパターンを探すことを称して“コードレビュー”という場合もあります。


content on this article is licensed under a Creative Commons License.

注目のテーマ