1つのコミットでどうすればよいかを考えるのではなく、一連のコミットでどうすればよいかを考えましょう。そうすれば、1つのコミットにどこまで含めればよいかを考えやすくなります。感覚的に言うと「コミットの流れを見ているだけでペアプログラミングしている気分になる」コミットが小さくまとまっているコミットです。ここをめざしてください。

これを支援するためにはどのような開発環境がよいのかについてはここでは省略します*1。

コミット単位の例

いくつか小さくまとまったコミットの具体例を紹介します。

インデントを直す

名前の変更やコードの移動などのリファクタリングをした後に変更したコードの周辺だけインデントが崩れることがあります。このようなときはインデントだけを直すコミットをします。

よいコミット:

diff --git a/lib/test/unit/pending.rb b/lib/test/unit/pending.rb
index 75cc8cb..75b1914 100644
--- a/lib/test/unit/pending.rb
+++ b/lib/test/unit/pending.rb
@@ -112,8 +112,8 @@ module Test
       def handle_pended_error(exception)
         return false unless exception.is_a?(PendedError)
         pending = Pending.new(name,
-                                filter_backtrace(exception.backtrace),
-                                exception.message)
+                              filter_backtrace(exception.backtrace),
+                              exception.message)
         add_pending(pending)
         true
       end

もし、まわりにtypoなどがあってもそれをこのコミットに含めてはいけません。ペアプログラミングをしているときのことを思い出してください。1度に1つの作業しかできませんよね。

また、複数のファイルや複数のクラスなど、変更が複数の塊にまたがる場合は別々のコミットにしましょう。ペアプログラミングをしているときは、インデントの修正でコードが壊れていないことを確認するために、それぞれの塊を修正するごとにテストを実行しますよね。

indentコマンドを使うなど、一括で機械的にインデントを直す場合は1つのコミットにまとめても構いません。ただし、そのときはコミットメッセージに実行したコマンドラインを残しておくとよいでしょう。

typoを直す

たくさんコードを書いているとtypoはよくあることです。typoを直すときは同じtypo毎にコミットをわけましょう。またコミットメッセージにどんなtypoを直したかも書いておくとdiffを見た人に親切です*2。

よいコミット:

diff --git a/lib/test/unit/notification.rb b/lib/test/unit/notification.rb
index 48ba3f6..c9c89b6 100644
--- a/lib/test/unit/notification.rb
+++ b/lib/test/unit/notification.rb
@@ -79,12 +79,12 @@ module Test
     module NotificationHandler
       class << self
         def included(base)
-          base.exception_handler(:handle_Notified_error)
+          base.exception_handler(:handle_notified_error)
         end
       end

       private
-      def handle_Notified_error(exception)
+      def handle_notified_error(exception)
         return false unless exception.is_a?(NotifiedError)
         notification = Notification.new(name,
                                 filter_backtrace(exception.backtrace),

typoの修正コミットに他の変更を混ぜるのはやめましょう。以下はtypoの修正とエラーメッセージの修正を1度にコミットしている悪い例です。

悪いコミット:

diff --git a/Rakefile b/Rakefile
index e3e73cf..bfcbe04 100644
--- a/Rakefile
+++ b/Rakefile
@@ -292,7 +292,7 @@ namespace :release do
       empty_options << "OLD_RELEASE_DATE" if old_release_date.nil?

       unless empty_options.empty?
-        raise ArgumentError, "Specify option(s) of #{empty_options.join(",")}."
+        raise ArgumentError, "Specify option(s) of #{empty_options.join(", ")}."
       end

       indexes = ["doc/html/index.html", "doc/html/index.html.ja"]
@@ -302,7 +302,7 @@ namespace :release do
          [old_release_date, new_release_date]].each do |old, new|
           replaced_content = replaced_content.gsub(/#{Regexp.escape(old)}/, new)
           if /\./ =~ old
-            old_undnerscore = old.gsub(/\./, '-')
+            old_underscore = old.gsub(/\./, '-')
             new_underscore = new.gsub(/\./, '-')
             replaced_content =
               replaced_content.gsub(/#{Regexp.escape(old_underscore)}/,

最初のhunkはjoinの引数にスペースを追加しているだけでtypoの修正ではありません。もし、コミットメッセージに「Fix typos」などと書かれていれば最初のhunkにもtypoがあるのではないかと思ってしまうでしょう*3。

名前をつける

マジックナンバーに名前をつけるときは1つのコミットで1つのマジックナンバーだけに名前をつけましょう。

以下は、C言語のプログラムの終了コードを0と-1というマジックナンバーから、EXIT_SUCCESSとEXIT_FAILUREという名前のついた値にするためのコミットです。もし、間違って0をEXIT_FAILUREに置き換えていても気づかないでしょう。

悪いコミット:

diff --git a/src/groonga.c b/src/groonga.c
index fca1755..d193d15 100644
--- a/src/groonga.c
+++ b/src/groonga.c
@@ -1938,10 +1938,10 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     wait(NULL);
-    return 0;
+    return EXIT_SUCCESS;
   }
   if (pidfile_path) {
     pidfile = fopen(pidfile_path, "w");
@@ -1951,7 +1951,7 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     if (!pidfile) {
       fprintf(stderr, "%d\n", pid);
@@ -1959,7 +1959,7 @@ do_daemon(char *path)
       fprintf(pidfile, "%d\n", pid);
       fclose(pidfile);
     }
-    _exit(0);
+    _exit(EXIT_SUCCESS);
   }
   {
     int null_fd = GRN_OPEN("/dev/null", O_RDWR, 0);
@@ -2587,7 +2587,7 @@ main(int argc, char **argv)
     line_editor_init(argc, argv);
   }
 #endif
-  if (grn_init()) { return -1; }
+  if (grn_init()) { return EXIT_FAILURE; }

   grn_set_default_encoding(enc);

しかし、以下のようにEXIT_SUCCESSへの置き換えとEXIT_FAILUREへの置き換えを別のコミットにしたらどうでしょうか。これなら間違って置き換えていても気づきやすいですね。ペアプログラミングをしているときでも、EXIT_SUCCESSへの置き換えとEXIT_FAILUREへの置き換えを同時にやっていると、ペアの人が間違いに気づきにくくなりますよね。

よいコミット1:

diff --git a/src/groonga.c b/src/groonga.c
index fca1755..2731006 100644
--- a/src/groonga.c
+++ b/src/groonga.c
@@ -1941,7 +1941,7 @@ do_daemon(char *path)
     return -1;
   default:
     wait(NULL);
-    return 0;
+    return EXIT_SUCCESS;
   }
   if (pidfile_path) {
     pidfile = fopen(pidfile_path, "w");
@@ -1959,7 +1959,7 @@ do_daemon(char *path)
       fprintf(pidfile, "%d\n", pid);
       fclose(pidfile);
     }
-    _exit(0);
+    _exit(EXIT_SUCCESS);
   }
   {
     int null_fd = GRN_OPEN("/dev/null", O_RDWR, 0);

よいコミット2:

diff --git a/src/groonga.c b/src/groonga.c
index 2731006..d193d15 100644
--- a/src/groonga.c
+++ b/src/groonga.c
@@ -1938,7 +1938,7 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     wait(NULL);
     return EXIT_SUCCESS;
@@ -1951,7 +1951,7 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     if (!pidfile) {
       fprintf(stderr, "%d\n", pid);
@@ -2587,7 +2587,7 @@ main(int argc, char **argv)
     line_editor_init(argc, argv);
   }
 #endif
-  if (grn_init()) { return -1; }
+  if (grn_init()) { return EXIT_FAILURE; }

   grn_set_default_encoding(enc);

モジュールの中に移動する

最初は単なるちょっとしたコードだったものが他のコードでも使いたくなるくらい便利なコードに育っていくことはよくあります。そのようなとき、ライブラリとして使えるようにモジュールに入れたりしますね。

例えば、以下のようなちょっとしたログ出力メソッドがあったとします。

123	def log(tag, message) puts("[#{tag}] #{message}")end

これをそのまま他のコードでも使おうとすると、トップレベルにlogメソッドが定義されてしまい、行儀がよくありませんね。このようなときは以下のようにモジュールの中に入れたりします。

module Logger
  module_function
  def log(tag, message)
    puts("[#{tag}] #{message}")
  end
end

このときは以下のように2つのコミットにわけます。

まず、モジュールで囲みます。しかし、まだ元のメソッドはインデントしません。

よいコミット1:

diff --git a/logger.rb b/logger.rb
index 1c7c4f0..7a3ed06 100644
--- a/logger.rb
+++ b/logger.rb
@@ -1,3 +1,6 @@
+module Logger
+  module_function
 def log(tag, message)
   puts("[#{tag}] #{message}")
 end
+end

次にモジュールの中身をインデントします。

よいコミット2:

diff --git a/logger.rb b/logger.rb
index 7a3ed06..293a335 100644
--- a/logger.rb
+++ b/logger.rb
@@ -1,6 +1,6 @@
 module Logger
 module_function
-def log(tag, message)
-  puts("[#{tag}] #{message}")
-end
+  def log(tag, message)
+    puts("[#{tag}] #{message}")
+  end
 end