View on GitHub

eng-practices

Google's Engineering Practices Documentation Japanese Translation

小さな CL

なぜ小さな CLを書くのか?

小さく、シンプルな CL とは、次のような CL のことです。

レビュアには、ただ CL が大きすぎるというだけの理由で無条件にリジェクトする決定権があるということを覚えておいてください。普通は、レビュアはあなたのコントリビューションに感謝し、どうにかして小さな一連の変更に分けられないかとお願いすることになるでしょう。あなたがすでに変更を書き終えた後に分割を行うのにはたくさんの仕事が必要になるかもしれませんし、大きな変更を受け入れるべき理由について議論するのに長い時間が掛かってしまうかもしれません。初めの段階で単に小さい CL を書いた方が簡単です。

小さいとはどのくらいか?

一般に、1つの CL の適切なサイズとは、1つの自己完結した変更のことです。つまり、次のような変更のことを指します。

どのくらいの大きさが「大きすぎるか」についての厳格なルールはありません。通常、1つの CL に対して 100 行のコードというのは妥当で、1000 行というのは大きすぎますが、最終的にはあなたのレビュアの判断に委ねられます。1 ファイルに対する 200 行の変更は問題ないかもしれませんが、200 行が 50 ファイルに渡って分散している場合は、通常は大きすぎます。

あなたはコードを書き始めた瞬間からそのコードに親しく関わっていても、レビュアにはそのコンテキストが分からないことがよくある、ということを心に留めておきましょう。あなたには受け入れ可能なサイズの CL だと思われても、レビュアには混乱を与えてしまうかもしれません。迷った場合には、自分が書く必要があると思うよりも小さい CL を書くようにしましょう。レビュアが CL が小さすぎると文句を言うことはまれです。

大きな CL はどのようなときに認められるのか?

次のように、大きな変更が悪くないとされる場合も少しだけあります。

ファイルを分割する

CL を分割するもう一つの方法は、自己完結した一連の変更を、別々のレビュアを必要とするファイルごとにグループ化することです。

一例としては、1つの CL でプロトコル・バッファの修正を行い、別の CL でその proto を使用するコードの変更を行うようにします。proto の変更の CL は、それを使用するコードの CL よりも先に送信しなければなりませんが、2つの CL を両方同時にレビューすることができるようになります。このように分割した場合、両方のレビュアたちにあなたが書いた他の CL の存在について知らせることで、レビュアたちがあなたの変更のコンテキストを知ることができるようにするといいでしょう。

別の例としては、1つの CL でコードの変更を行い、別の CL でそのコードを使用する設定や検証を行うようにします。こうすることで、必要があったときにより簡単にロールバックできるようになります。設定や検証のためのファイルは、コードの変更よりも素早く本番環境に投入できるからです。

リファクタリングを分離する

通常、リファクタリングは、機能の変更やバグフィックスとは独立した CL で行うのが最善です。たとえば、クラスの移動や名前の変更は、そのクラス内のバグの修正とは別の CL で行うべきです。CL が分かれていれば、レビュアは各 CL で行われた変更をずっと簡単に理解できるようになります。

ローカル変数名の変更のような小さなクリーンアップであれば、機能の変更やバグフィックスの CL に含めることができる場合もあります。リファクタリングが大きくなって現在の CL に含めた場合にレビューが難しくなるのはいつなのか、その判断は、開発者とレビュアに委ねられています。

関連するテストコードを同じ CL の中に含める

テストコードを別の CL に分離するのは避けてください。あなたのコードの変更を検証するテストは、たとえコードの行数が長くなってしまうとしても、同じ CL に含めなければなりません。

しかし、独立したテストの修正は、最初に1つの CL として分離することができます。これは、リファクタリングのガイドラインに似ています。CL の分離は、次のような手順で行います。

ビルドを壊さない

互いに依存する複数の CL がある場合、各 CL が提出された後でもシステム全体が確実に機能し続けるようにする方法を探す必要があります。そうしなければ、あなたが CL を提出中の数分間、仲間の開発者たち全員のビルドを壊してしまう可能性があります (あるいは、後で提出した CL が予期せず誤っていた場合、更に長い時間がかかってしまいます)。

十分小さくできない

時には、CL を大きくするしかないと思われるような状況に出会うことがあるかもしれません。しかし、本当にそうするしかない場合は極めて稀です。小さな CL を書くことを実践している作者たちは、ほとんど必ず、機能を一連の小さな変更に分解する何らかの方法を見つけることができます。

大きな CL を書く前に、先にリファクタリングのみの CL を書くことで、よりクリーンな実装への道が開けないかどうか、よく考えてみてください。チームメイトと話をして、その機能を代わりの小さな CL で実装する方法についてアイデアがある人を探してみてください。

これらのすべての選択肢を行っても、本当にどうしても良い方法が見つからない場合には (そのような場合は本当に極めて稀であるはずです)、大きな CL をレビューすることについてあなたのレビュアたちからコンセンサスを得て、これから行うことをレビュアに警告するようにします。大きな CL のレビューでは、長い時間をかけてレビューのプロセスを完全に行い、バグが入り込まないように十分警戒し、入念に追加のテストを書くことが期待されます。

Next: レビュアのコメントの扱い方