ja ja ja ja ja ja ja ja ja en ja ja ja ja ja ja ja ja ja ja ja ru ja ja ja ja ja ja ja ja ja ja ja ja ja ja en ja ja ja ja ja ja en ja ja en ja ja ja ja ja en ja ja en ja ru ja ja ja zh ja zh ja ja ja en ja ja zh ja ja ja ja ja ja ja ja ja ja ja ja ja ja ja en ja ja ja en ja ja en ja ja vi ja sv eu ja en en hu ja ja en en pl ja ja jbo hbs_lat en ja ja ja ja ja ja ja ja ja ja ja ja ja ja ja ja ja ja ja
もし、まわりにtypoなどがあってもそれをこのコミットに含めてはいけません。ペアプログラミングをしているときのことを思い出してください。1度に1つの作業しかできませんよね。
また、複数のファイルや複数のクラスなど、変更が複数の塊にまたがる場合は別々のコミットにしましょう。ペアプログラミングをしているときは、インデントの修正でコードが壊れていないことを確認するために、それぞれの塊を修正するごとにテストを実行しますよね。
indentコマンドを使うなど、一括で機械的にインデントを直す場合は1つのコミットにまとめても構いません。ただし、そのときはコミットメッセージに実行したコマンドラインを残しておくとよいでしょう。
たくさんコードを書いていると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への置き換えを同時にやっていると、ペアの人が間違いに気づきにくくなりますよね。
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);
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);
モジュールの中に移動する
最初は単なるちょっとしたコードだったものが他のコードでも使いたくなるくらい便利なコードに育っていくことはよくあります。そのようなとき、ライブラリとして使えるようにモジュールに入れたりしますね。
例えば、以下のようなちょっとしたログ出力メソッドがあったとします。
def log(tag, message) puts("[#{tag}] #{message}") end
これをそのまま他のコードでも使おうとすると、トップレベルにlogメソッドが定義されてしまい、行儀がよくありませんね。このようなときは以下のようにモジュールの中に入れたりします。
module Logger module_function def log(tag, message) puts("[#{tag}] #{message}") end end
このときは以下のように2つのコミットにわけます。
まず、モジュールで囲みます。しかし、まだ元のメソッドはインデントしません。
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
次にモジュールの中身をインデントします。
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
このように分けることで、たとえ一緒に同じ作業をしていなくても、一連のコミットを見るだけで何をしようとしているかが伝わります。
1つのコミットのことだけを考えていると同時にコミットしたくなりますが、一連のコミットを考えるとこのように表現することもできます。意図が伝わるコミットです。
コミットの内容を小さくまとめるにはどうしたらよいかの指針とその具体例をいくつか紹介しました。
1つ1つのコミットの積み重ねでクリアなコードが作られていきます。もちろん、1つのコミットは大切にしますが、一連のコミットも大切にして、意図が伝わるコミットにしましょう。コミットを見ることで、チームのみんなにどのように開発しているかが伝わるようなコミットにしていきましょう。
なお、フリーソフトウェアの開発のように、世界中の様々な場所・様々な時間に開発が行われているような場合はこのような意図が伝わるコミットのしかたがより重要になります。信頼されるようなコミットを重ねていきましょう。
例えば「diff入りのコミットメールを送る」という方法があります。 ↩
余談ですが、typoを直すコミットメッセージ中でtypoすることはよくある話です。 ↩
「Fix a typo」なら最初のhunkにはないと思うかもしれません。しかし、「そしたら最初のhunkはなんだろう?」ということになるのでそれでもよくありません。 ↩
現在クリアコードではソフトウェア開発者を募集しています! フリーソフトウェア開発での開発の仕方をベースに、成果や知識を公開し共有しながら業務に携われます。
The Art of Readable Code
Emacs実践入門 - おすすめEmacs設定2012
Apache Arrow
milter manager
ククログの記事はCC BY-SA 4.0とGFDLのデュアルライセンスで自由に利用できます。
クリアコードはプログラミングが楽しいソフトウェア開発者を1名募集しています。
クリアコードは「クリアコードをいい感じにする人」を1名募集しています。
クリアコードはフリーソフトウェア開発で培った技術力を提供しています。特にMozilla製品(Mozilla FirefoxとMozilla Thunderbird)とRubyとGroonga(全文検索)に関連した開発を得意としています。