No Longer My Favorite Git Commit

Michael Lynch

もはや私のお気に入りではないGitコミット

原文は Michael Lynch により に公開されました。 このブログを購読する

6年前、David Thompsonは「My favourite Git commit」という人気のブログ記事を書きました。同僚が書いた、気まぐれなまでに詳細なコミットメッセージを称賛する内容でした。当時私はその記事を気に入り、良いコミットメッセージの手本として何人ものチームメイトに共有してきました。

最近、自分なりの役に立つコミットメッセージの書き方ガイドを作成するにあたって、Thompsonの記事を読み返しました。Thompsonの記事の何がそこまで優れた例なのか説明を求められたとき、驚いたことにうまく答えられませんでした。部外者として読む分には面白いのですが、優れたソフトウェアエンジニアリングの手本として正当化することはできなかったのです。

Thompsonのお気に入りのコミット

当時Thompsonをはじめ、私を含む多くの人を魅了したコミットメッセージがこちらです。

Convert template to US-ASCII to fix error

featureブランチで、/etc/nginx/router_routes.conf の内容を検証するテストを追加しました。bundle exec rake specbundle exec rspec modules/router/spec で実行すると問題なく通りました。しかし bundle exec rake で実行すると、各 should ブロックが次のエラーで失敗しました。

ArgumentError:
 invalid byte sequence in US-ASCII

最終的に、.with_content(//) マッチャーを削除するとエラーが消えることがわかりました。specファイルには変な文字はないようでした。そして、同じインタプリタでPuppetをrequireすることで再現できることもわかりました。

rake -E 'require "puppet"' spec

その特定のテンプレートだけが、コードベース内でエンコーディングが utf-8 と判定される唯一のファイルのようです。他はすべて us-ascii でした。

dcarley-MBA:puppet dcarley$ find modules -type f -exec file --mime {} \+ | grep utf
modules/router/templates/routes.conf.erb:  text/plain; charset=utf-8

そのファイルをUS-ASCIIに変換しようとしたところ、問題の文字は空白のように見える何かであることがわかりました。

dcarley-MBA:puppet dcarley$ iconv -f UTF8 -t US-ASCII modules/router/templates/routes.conf.erb 2>&1 | tail -n5
  proxy_intercept_errors off;

  # Set proxy timeout to 50 seconds as a quick fix for problems
  #
iconv: modules/router/templates/routes.conf.erb:458:3: cannot convert

(手作業で)置換した後、ファイルは再び us-ascii と判定されるようになりました。

dcarley-MBA:puppet dcarley$ file --mime modules/router/templates/routes.conf.erb
modules/router/templates/routes.conf.erb: text/plain; charset=us-ascii

これでテストが通るようになりました!返ってこない1時間を費やしてしまいました……。

この長い前置きのあと、Thompsonが実際のdiffを示すところが「オチ」です。

そうです、このコミットメッセージは、たった1文字の空白の変更を説明するために、6つの段落と5つのコードスニペットで構成されているのです。

お気に入りと最高は別物

このコミットが魅力的な理由は簡単に理解できます。

ほとんどの開発者ならこの変更を単に「Fix whitespace character」とだけ書くでしょう。だからこそ、バグの調査と修正の過程をわざわざここまで詳しく説明している点に、うれしい驚きがあるのです。

Thompsonが挙げる理由から見れば、これは良いコミットメッセージです。検索可能な成果物を作り、開発者のツールやプロセスに関する有益な洞察を共有しているからです。

これはThompsonや、ましてや元のコミットの作者への批判ではありません。Thompsonはこれが「最高」のコミットメッセージだと言ったわけではなく、単に「お気に入り」だと言っただけなのです。

とはいえ、今ではこれを模範的なコミットメッセージとして使うことを妨げる欠点が見えてきました。

最も重要な情報が最後に埋もれている

Thompsonが最初にブログ記事を公開したとき、最も多かった批判の一つは、コミットメッセージが冗長すぎるというものでした。私はその批判は的外れだと思っていました。

コミットメッセージの詳細が徹底していることは、関連性がある限り有用です。Thompsonの例もそうでした。経験の浅いチームメイトにとっては作者のデバッグ過程やツールセットを学ぶ助けになりますし、経験豊富なチームメイトにとっては、開発者が何かを見落としていないか、あるいは有用なツールを知らないのではないかを確認する機会にもなります。

人々がThompsonの例を冗長だと感じた理由は、最も重要な情報がコミットメッセージの奥深くに埋もれているからです。

最初の段落を読み返してみましょう。

featureブランチで、/etc/nginx/router_routes.conf の内容を検証するテストを追加しました。bundle exec rake specbundle exec rspec modules/router/spec で実行すると問題なく通りました。しかし bundle exec rake で実行すると、各 should ブロックが次のエラーで失敗しました。

ArgumentError:
 invalid byte sequence in US-ASCII

コミットメッセージに入って3文と1つのコードスニペットを読んでも、読者はいまだにこの変更が実際に何をするのかまったくわかりません。

コミットメッセージでは、最も重要な情報を最初に提示し、徐々に細部へと移行すべきです。ジャーナリストはこれを逆ピラミッド型の書き方と呼びます。

逆ピラミッド

ジャーナリストはニュース記事を逆ピラミッド構造で構成します。最も多くの人に関係する情報が一番上に来るようにするのです。

コミット履歴をスクロールしているときは、各コミットが自分に関係あるかどうかを素早く知りたいものです。コミットメッセージは、冒頭から変更点の概要を高レベルで示すべきなのです。

問題点がはっきり説明されていない

Thompsonの例のコミットメッセージを最後まで読んで、あなたは変更内容を理解できただろうか。

問題の説明に最も近い部分はここです。

その特定のテンプレートだけが、コードベース内でエンコーディングが utf-8 と判定される唯一のファイルのようです。他はすべて us-ascii でした。

dcarley-MBA:puppet dcarley$ find modules -type f -exec file --mime {} \+ | grep utf
modules/router/templates/routes.conf.erb:  text/plain; charset=utf-8

メッセージでは routes.conf.erb がUTF-8エンコーディングであると述べられていますが、なぜそうなっているのかは説明されていません。幸い、このプロジェクトはオープンソースなので、自分で調べてみることにしました。

問題は 463行目routes.conf.erb にありました。

$ cat modules/router/templates/routes.conf.erb | head -n 463 | tail -n 1
  # where civica QueryPayments calls are taking too long.

通常のテキストエディタやウェブブラウザでは問題は見えませんが、xxd のようなツールでファイルの生バイトをダンプすると、問題が浮かび上がります。

$ cat modules/router/templates/routes.conf.erb \
  | head -n 463 | tail -n 1 \
  | xxd | head -n 1
00000000: 2020 23c2 a077 6865 7265 2063 6976 6963    #..where civic
                 ^^ ^^

US-ASCIIとUTF-8のテーブルを暗記していない人のために、その行の最初の数文字を整理すると次のようになります。

バイト表現テキスト表現
0x20' ' (space)
0x20' ' (space)
0x23'#'
0xC2 0xA0' ' (UTF-8のノンブレーキングスペース)

つまり、このファイルにはバイト列 0xC2 0xA0 が含まれていたため、US-ASCIIファイルではあり得ません。0xC20xA0 も、どちらもUS-ASCIIのバイト範囲外だからです。

0xC2 0xA0 という並びは、routes.conf.erb を読み込むあらゆるアプリケーションが、テキストをエンコードするより新しく、より国際的に対応した方式であるUTF-8エンコーディングで解釈しなければならないことを意味します。

Thompsonのコードベースでは Ruby 1.9.3 が使われていました。これはUTF-8エンコーディングをサポートしていましたが、ファイルが明示的に宣言していない場合はデフォルトでUS-ASCIIとして扱われました。

ソースの履歴を掘り下げると、UTF-8文字を最初に持ち込んだのはコミット 5a8607 であることがわかりました。そのコミットメッセージにはUTF-8文字を導入した理由が一切書かれていないので、おそらく偶発的なものだったのでしょう。

あるHacker Newsのコメント投稿者は、なぜその迷子のUTF-8文字が routes.conf.erb に紛れ込んだのかについて、もっともらしい説を提示していました。

無効な文字の出所として考えられるのは、アイルランド/英国配列のAppleキーボードを使っていた誰かが、# を Option-3(AltGr-3)で入力し、ノンブレーキングスペースを Option-Space(AltGr-Space)で入力してしまったことです。

-Hacker Newsのmesse

コードに言及しているのにリンクがない

Thompsonの例のコミットは、外部のコードへの言及から始まります。

featureブランチで、/etc/nginx/router_routes.conf の内容を検証するテストを追加しました。bundle exec rake specbundle exec rspec modules/router/spec で実行すると問題なく通りました。

しかしコミットメッセージではブランチ名もコミットハッシュも示されていないため、読者は開発者の発見を再現する手段がありません。

コミットメッセージの後半では、こう述べられています。

最終的に、.with_content(//) マッチャーを削除するとエラーが消えることがわかりました。specファイルには変な文字は見当たりませんでした。

コミットハッシュやリンクがなければ、読者はどのマッチャーやどのspecファイルのことを指しているのかわかりません。

コミットメッセージが外部のコードに言及するなら、明示的にリンクすべきです。そうすれば、コードレビュアーや将来のメンテナが変更の正確な文脈を確認できます。

私なりの書き直し

Thompsonのお気に入りのGitコミットを、私なりに書き直した案がこちらです。

Convert routes.conf.erb template to US-ASCII

routes.conf.erb には、5a8607 で誤って混入したと思われる迷子のUTF-8文字があります。

rake はUS-ASCII形式を想定しているため、routes.conf.erb 内のたった1つのUTF-8文字が rake でのテスト失敗を引き起こします。

この変更では、rake でのテスト失敗を防ぐため、UTF-8文字を同等のUS-ASCII文字に置換します。

迷子のUTF-8文字

問題は modules/router/templates/routes.conf.erb の463行目にあります。

$ cat modules/router/templates/routes.conf.erb \
  | head -n 463 | tail -n 1 \
  | xxd | head -n 1
00000000: 2020 23c2 a077 6865 7265 2063 6976 6963    #..where civic
                 ^^ ^^

0xC2 0xA0 は有効なUS-ASCIIのバイト列ではありません。これはUTF-8のノンブレーキングスペース文字です。US-ASCIIエンコーディングを期待してファイルを読み込むツールは、いずれも失敗します。

どのように発見したか

featureブランチで /etc/nginx/router_routes.conf の内容を検証するテストを追加しました(abcd123 を参照)。bundle exec rake specbundle exec rspec modules/router/spec で実行したときは問題なく通りましたが、bundle exec rake としてテストを実行すると、各 should ブロックが次のエラーで失敗しました。

ArgumentError:
 invalid byte sequence in US-ASCII

最終的に、.with_content(//) マッチャーを削除するとエラーが消えることがわかりました。specファイルには変な文字は見当たりませんでした。同じインタプリタでPuppetをrequireすることで、エラーを再現できました。

rake -E 'require "puppet"' spec

その特定のテンプレートだけが、コードベース内で file コマンドが utf-8 と判定する唯一のファイルのようです。他はすべて us-ascii でした。

$ find modules -type f -exec file --mime {} \+ | grep utf
modules/router/templates/routes.conf.erb:  text/plain; charset=utf-8

そのファイルをUS-ASCIIに変換しようとしたところ、問題の文字は空白のように見える何かであることがわかりました。

$ iconv -f UTF8 -t US-ASCII modules/router/templates/routes.conf.erb 2>&1 \
   | tail -n5

  proxy_intercept_errors off;

  # Set proxy timeout to 50 seconds as a quick fix for problems
  #
iconv: modules/router/templates/routes.conf.erb:458:3: cannot convert

(手作業で)UTF-8文字を置換した後、file コマンドは routes.conf.erb を再び us-ascii と判定するようになりました。

$ file --mime modules/router/templates/routes.conf.erb
modules/router/templates/routes.conf.erb: text/plain; charset=us-ascii

これでテストが通るようになりました!返ってこない1時間を費やしてしまいました……。

私が行った変更点は次のとおりです。

  • メッセージの冒頭に高レベルな概要を追加しました。
  • UTF-8文字についてのより明確な説明と、その由来を追加しました。
  • 作者の元の内容の大部分を「どのように発見したか」セクションに移し、それが発展的な読み物であることを明確にしました。
  • 軽微な文法修正を行いました。
  • 曖昧さを減らすため、受動態を排除しました
  • ターミナルのプロンプトを dcarley-MBA:puppet dcarley $ から単なる $ に簡略化しました。前者はほとんどノイズでしかないためです。

特筆すべきは、詳細を削除しなかったことです。問題は冗長さではなく、開発者が情報をどのように整理し提示したかにあったからです。

自分自身の原則を定義することの価値

Thompsonの記事を読み返すことで、ソフトウェアエンジニアリングの原則を自分自身で定義することにどれだけ価値があるかを改めて思い出しました。

私はThompsonの長所についての意見に同意していたため、このコミットを良い例として受け入れていました。コミットメッセージにおいて最も重要だと考える資質を自分で腰を据えて定義してみるまで、Thompsonの例の欠点には気づかなかったのです。

私はこれまでにも、さまざまなソフトウェアエンジニアリングのプラクティスについて自分の視点を説明してきましたが、そのたびにより良い開発者になれたと感じています。当たり前だと思い込んでいた考えを批判的に見つめ直すことを強いられ、常に達成できるとは限らなくても、自分の理想がどのようなものかを思い出させてくれるからです。

関連記事


govuk-puppetプロジェクトからの抜粋は、Copyright Crown Government Digital Service、MIT License の下で使用しています。

この記事は「muse-spark-1.2-contributor」を使用して翻訳されました。

コメント