ダメなコードを防ぐためのあれこれ #34

公開日: 2012-04-01


ダメなコードというものは困りものだ。ダメなコードが一度入ってしまうと、それを取り除くのは難しいし、ダメなコードはガンのように転移して広がってしまうものだ。というわけで、ダメなコードは入ってしまう前に防ぐのが肝要だ。

私が関わっている Chromium プロジェクト[1] では、様々な手法を使ってこの問題に取り組んでいる。

一番効果的なのは、チェックイン前のコードレビューだ。ダメなパッチは丁重にお断りして、修正が必要なパッチはチェックインする前に直してもらう。チェックインしてから直す、というやり方だと、忙しいとかなんとかという理由で、結局そのままになってしまいがちなので、よろしくない。

コードレビューは有用だが、不適切な人がレビューをしたのではあまり意味がない。プロジェクトの規模が大きいと、全体を把握している人はほとんどいないので、変更部分のコードに一番詳しい人にレビューを頼むのが好ましい。Chromium では OWNERS ファイル [2] という仕組みを使って、コードのオーナーを明確にしている。オーナーからLGTM (Looks Good To Me) をもらわないと、チェックインできない仕組みだ。

コードレビューでは、コードのデザイン、実装、テストなどのチェックに専念したい。瑣末なコーディングスタイルの間違いなどを人間がチェックするのはばからしい。こういうのはツールの出番だ。GoogleのC++スタイル[3]なら cpplint.py というツールが使える。が、せっかくツールがあっても、手動だと実行するのを忘れてしまいがちだ。

Chromium では PRESUBMIT スクリプト[4] という仕組みを使って、パッチをアップロードするタイミングで cpplint.py を走らせたり、その他もろもろのチェックを自動で行なっている。ワークフローに組み込まれているので、実行をし忘れるということがない。

コードをチェックインする前には、ビルドが通って既存のテストが通るかを確認したい。が、Chromium のように Windows, Mac, Linux などの複数のプラットフォームで動くプロジェクトでは、手元にすべての環境を揃えてテストするのは現実的ではない。そこで、Chromium では try サーバ [5] という、チェックインする前に、サーバ上でビルドしてテストを実行させる仕組みが提供されている。

こうした経緯を経て、ようやくチェックインしたコードは buildbot [6] という継続ビルドシステムによってビルドされテストが実行される。ビルドまたはテストを壊したたパッチは tree sheriff [7] というビルドを見張るその日の担当者によって取り除かれる。

チェックインをより安全にするために、tryサーバでのビルドとテストを行ったのちにチェックインするという commit queue [8] というシステムが昨年から導入された。

このように Chromium プロジェクトでは、ダメなコードを防ぐために膨大なコストをかけている。が、ここまでやってもコードの品質を保つのは難しく、年がら年中、大規模なリファクタリングが行われている。こういうダイナミズムは Chromium プロジェクトのいいところのひとつだ。

[1] http://dev.chromium.org/Home
[2] http://dev.chromium.org/developers/owners-files-1
[3] http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml
[4] http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
[5] http://dev.chromium.org/developers/testing/try-server-usage
[6] http://dev.chromium.org/developers/testing/chromium-build-infrastructure/tour-of-the-chromium-buildbot
[7] http://dev.chromium.org/developers/tree-sheriffs
[8] http://www.chromium.org/developers/testing/commit-queue

Satoru Takabayashi