Money Forward Developers Blog

株式会社マネーフォワード公式開発者向けブログです。技術や開発手法、イベント登壇などを発信します。サービスに関するご質問は、各サービス窓口までご連絡ください。

20230215130734

レビューしやすいコミット履歴でバグ削減

こんにちは。 アグリゲーション開発担当の中川です。

今回は、みんなが大好きな構成管理ツール「Git」について話したいと思います。

私は Git を使い始めてから、バグの発生数が激減しました。 Git を使ったとある手法によってレビューが充実し、バグの少ないコードを書くようになったと考えています。

では、今回はその手法について紹介したいと思います。

※ 本稿は Git 以外の第三世代構成管理ツール(Hg、Bzr など)にも適用するかと思いますが、Git の用語とコマンドを使って紹介していくため Git の基本知識が必要となります。ご了承ください。  

レビューしやすいコミット履歴と、開発の流れで自然にできるコミット履歴の乖離

以下のようなコミット履歴があるとします。

1. wip: 仕様変更○○を行い始めた
2. wip: 仕様変更○○の続き
3. wip: ちょっと設計を変更、それと過去のバグを見つけたのでついでに修正
4. wip: やっぱり設計はこっちが良い、変数と関数名も少し変更
5. ある程度出来たので一度コミット
6. バグったので修正
7. ちょっとやり方を改善
8. バグったので修正
9. バグったので修正

これは、とある開発者のA君が水曜日の午前中をかけて仕上げた改修のコミット履歴です。

A君は、普通にコーディングしながら定期的にコミットを行った結果、このようなコミット履歴が出来上がりました。

A君の作業の流れがそのままコミットに反映されているこのコミット履歴は、どこに問題があるのでしょうか?

答えは、レビューの観点にあり、レビュアーにとってはA君の作業の流れは特に重要でなく、システムに対して加えられる変更を知ることが最も重要だからです。 その上、一実装者の改修を数人のレビュアーがレビューするチームでは、レビュアーの時間は貴重な資源となり、レビューしやすいコミットを作ることが大切になってきます。

しかしながら、コミット履歴は自然な開発の流れでこの並びになったのであって、初めからレビューしやすいコミット履歴を作りながら開発することは困難です。無理にそのようなことを意識して開発を行えば、発想を妨げ、かえって非効率になりがちです。

では、A君はどのようにするば良いのでしょうか?

結果的に、A君は上のコミットをそのままレビューに出さず、このようにしました。

1. 機能 X のバグを修正 (過去のバグを見つけたので修正した)
2. 機能 Y で冗長なコードをリファクタ (仕様変更○○に必要な機能追加に備えてコードをリファクタ)
3. モジュール W に XYZ 機能を追加
4. 新モジュール M を追加 (単体に分けられる機能は独立してコミット。テストと一緒にコミットするのが理想)
5. 仕様変更○○

これは、A君が水曜日の午後に数十分かけて整理したコミット履歴です。

このように改修目的がまとまった単位で整理されたコミット履歴を pristine commit history(綺麗な履歴)と言うようです。

昔の構成管理ツール(例えば SVN)は、コミットを後から変更すると言う概念が無かったため、作業の流れがそのままのコミットとなった履歴をメインブランチに出すしかありませんした。 しかし、git など第三世代の構成管理ツールは、コミットを作った後にそれを変更するというワークフローが配慮されているため、品質管理では貴重なツールとなっています。  

変更可能なコミット履歴の範囲

コミット履歴は、共通ブランチにマージしてプッシュするまでが変更可能な範囲です。

Clipboard03 こちらの図では、コミット AB はチームで共有している master にマージされ origin に展開されているため変更はできません。 CD はまだローカルの my_branch にしか無いため変更可能であり、この間に置いてはコミットの変更、分離、削除、挿入、並び替えが可能です。  

コミットは議論性の低い順番に並べる

基本的な考え方として、レビューに出すコミットの並びは議論性の低い順番に整列するのが良いと言われています。

具体的には、このような並びが良いでしょう。 (それぞれの分類で複数コミットが存在しても構いません。)

1. 既存バグの修正
  ↓
2. 機械的なリファクタリング
  ↓
3. リファクタリング
  ↓
4. 単独モジュールの追加(後続の改修で用いられるもの)
  ↓
5. 共通基盤の拡張
  ↓
6. 仕様変更

レビューする人は、後の方のコミットに最も時間をかけて慎重に確認することができます。また、指摘を受けやすい議論性の高いコミットを後の方に置くことで更なる変更が加えやすくなります。

レビューに出した仕様変更が最終的に無しになる場合でも、「既存バグの修正」はシステムにあった方が良いもので、「リファクタリング」や「単独モジュールの追加」もコードベースを良くする物であれば入れることに越したことがありません。

このように、議論性が低く採用しやすいコミットを先に置くことで、必要に応じて途中までのコミットをそのままメインブランチにマージすることが可能となります。

尚、これは後続のコミットが先方のコミットに依存している場合の話です。依存しない場合は、そもそも別のブランチで管理するのが良いでしょう。

[イメージ] 依存し合わないコミットは別のブランチで管理

                      bar
                       ↓
           T --> U --> V
     /
  ----> X --> A --> B --> C --> D
        ↑                       ↑
  origin/master                foo

後続が先方に依存する ADTV は、互いに依存しないため foo 改修と bar 改修に分けます。  

Git を活かす作業フロー

ここでいきなり基礎的な話になりますが、綺麗な履歴作りの前提となるためおさらいを兼ねて見ていきましょう。  

インデックスを有効活用しましょう

コミットの際、対象ファイルだけを選択し、コミットメッセージを記入してコミットしていませんか?

インデックスを活用する方法では、ファイルを一度インデックスに「登録」し、インデックスの diff を確認してから、コミットメッセージを記入してコミットする方法となります。 この方法は Git の強みを活かすために最もお勧めです。

インデックスは「ステージング」や「ステージングエリア」とも言います。コミットを行う際に用いる機能としては後者の用語を使うのが主流です。

コミット時にステージングを使わない方法

  1. 対象ファイルを一式選択
  2. コミットメッセージを記述してコミット

コミット時にステージングを有効に使う方法

  1. ステージングの準備
    1. 対象ファイルをステージングに登録
    2. ステージングの内容を確認
    3. コミットに収める改修が揃うまで A ~ B を繰り返す
  2. コミットメッセージを記述してコミット

ステージングに入れて、確認する工程を設けることで、コミットに含まれる改修をより明確に意識することが可能となります。

[追記] GUI 系のツールの中には Git のインデックス/ステージングが扱えないツールもありますので気を付けましょう。  

コミットは行や「ハンク」単位でステージングして作成

前章ではファイルをステージングに登録してからコミットを準備するという話をしましたが、実際はファイル単位でなく、行又は「ハンク」単位でステージングする事を推奨します。

[イメージ] ステージングにハンクの登録 (git-gui 版)

Clipboard01

git-gui では橙色(左上)の "Unstaged Changes" (未ステージ)ペインからファイルを選択し、黄色(右)のペインからハンクや行を選択して登録します。

すると、登録したハンクや行が未ステージの表示から消えます。緑色(左下)の "Staged Changes (Will Commit)" のペインのファイルを選択すると、前述でステージングに登録した差異が黄色のペインに表示されます。

ステージングに入れすぎた場合は、緑色のペインからファイルを選択して、同じように黄色のペインからハンクや行を選択してステージングから外すことも可能です。

[イメージ] コマンドラインからハンクをステージング

Clipboard02

git add -p で追加する「ハンク」を選択します。git reset -p でステージングから下す「ハンク」を選択します。git diff --cached でステージングされている差異を確認します。(add -preset -p の時は e を利用すると、行単位以下でもステージング対象にすることができます。)

ステージングを活用するメリット

一つのファイルに複数の用途の改修が入っている場合、ステージングを使わなければ一コミットに混ざってコミットされてしまいますが、部分的にステージングしてコミットすることで個別のコミットを分けることが可能となります。

他にこのようなメリットがあります。

  • ステージングするハンクを意識して選択することで自然とコードが読み直される
  • ファイル単位で diff を一式見直すよりも問題点を見落としにくい
  • 誤った改修に気づいた場合はその場で直したり、ステージングしないで残していける  

綺麗な履歴作り

さて、A君のコミット履歴ですが、A君が行ったことは主に次の三点です。

  1. 同じ機能のコミットは一つのコミットに統合して冗長性を無くす
  2. 分けられる機能が混ざっているコミットは分割して、それぞれの機能ごとの個別コミットを作る
  3. コミットを議論性の低い順番に並び替える

これらの変更を加えるためのコマンドを紹介します。  

用語 (HEAD、デタッチ状態)

コマンド紹介の先に用語のおさらいです。

現在「居る」コミットを示します。(多くの Git コマンドは HEAD を対象に実施されます。)

例えば、

git checkout foo を打つと、HEADfoo ブランチとなます。

  ----> X --> A --> B --> C
        ↑                 ↑
  origin/master          foo <- HEAD

ここから新しくコミットを作ると、foo は新コミットに進み、同時に HEAD も移動します。

  ----> X --> A --> B --> C --> D
        ↑                       ↑
  origin/master                foo <- HEAD

[ポイント] 基本的に HEAD の場所は git checkout &lt;commit>git reset &lt;commit> のコマンドで操作します。(どちらも第三引数の &lt;path> 引数を指定すると HEAD 操作ではない仕様となりますが。)

上の例に続いて git checkout HEAD~~ を行うと、HEAD~ の数分(二つ)前の B に移り、そこから新たなコミットを作成すると HEAD もそのコミットに移動します。

                          HEAD
                           ↓
                      .--> E
                     /
  ----> X --> A --> B --> C --> D
        ↑                       ↑
  origin/master                foo

 

デタッチ状態

HEAD がブランチを指していない状態を示します。

基本的に git checkout &lt;branch> を行うと HEAD&lt;branch> のブランチとなりますが、コミットID の直接指定や --detach オプションを使った場合は「デタッチ状態」となり、HEAD はブランチでなくコミットそのもの指す状態となります。(前章の例で E を指している HEAD もデタッチ状態です。)

尚、デタッチ状態で作ったコミットは、ブランチ名が無いため別ブランチに移った後元のコミットに戻るには「コミットID」の直指定が必要となります。また、ブランチ/タグが無く HEAD が離れているコミットは数日経つと git gc によって消滅します。このため、デタッチ状態で行うコミットは後からブランチを付ける前提で作るか、使い捨てで作るものに限ります。  

コマンド紹介: コミットの削除、分離と統合 (cherry-pick を使った方法)

続く削除、分離、統合のコマンドの例は全てこの状態から始めます。

  ----> X --> A --> B --> C --> D
        ↑                       ↑
  origin/master                foo <- HEAD

まずは B を削除する方法

# 1. まずベースに戻す (origin/master はブランチ(ローカル)ではないのでデタッチ状態となる)
git checkout origin/master

# 2. B 以外のをチェリーピック
git cherry-pick A C..D

# 3. 任意: 元の状態と比較 (B の撤去だけが差異として表示されるはず)
git diff foo..

# 4. ブランチを付け替え
git checkout -B foo

結果、

                            foo <- HEAD
                             ↓
          .--> A' --> C' --> D'
         /
  ----> X --> A --> B --> C --> D
        ↑
  origin/master

B が無い派生履歴が出来上がります。

[ポイント] チェリーピックする際にコミットを指定する順番を変えると新履歴の並びも変わります。  

次に、B の内容をコミット B1, B2 に分離する方法

# 1. HEAD を B に移す (指定は B のコミットID です)
git checkout B

# 2. B1 に要らないハンク/行をインデックスから下す
git reset -p HEAD~

# 3. amend コミットで HEAD の B を B1 に書き換える
git commit --amend -m "B1 のコミットメッセージ"

# 4. インデックスから下されていたハンクを B2 用にステージングしてコミット
git add -p
git commit -m "B2 のコミットメッセージ"

# 5. C, D を普通にチェリーピック
git cherry-pick C..D

# 6. 任意: 元の状態と比較 (分離しただけなので差異が無いはず)
git diff foo..

# 7. ブランチを付け替え
git checkout -B foo
  • 2 ~ 3 は git-gui で "Amend Last Commit" を使うと同じ結果になるので、それを使うのが便利です。
  • 4 は普通にコミットを作るツールで行えます。

結果、

                                         foo <- HEAD
                                          ↓
                .--> B1 --> B2 --> C' --> D'
               /
   ---> X --> A --> B --> C --> D
        ↑
  origin/master 

C と B を一つのコミットに統合する方法

# 1. HEAD を C に移す (指定は C のコミットID です)
git checkout C

# 2. ワーキングツリーとインデックスの内容を変えずに HEAD を B に移動
git reset --soft B

# 3. B を更新するコミット (インデックスは C の状態を引き継いだのでステージング済み同様です)
git commit --amend -m "B + C のコミットメッセージ"

# 4. D をチェリーピック
git cherry-pick D

# 5. 任意: 元の状態と比較 (統合しただけなので差異が無いはず)
git diff foo..

# 6. ブランチを付け替え
git checkout -B foo
  • 1 ~ 2 は Bcheckout してから Ccherry-pick --no-commit しても同じです。

結果、

                           foo <- HEAD
                            ↓
                .--> BC --> D'
               /
   ---> X --> A --> B --> C --> D
        ↑
  origin/master 

 

履歴の書き換え (rebase -i を使った方法)

上では cherry-pick を使った方法を紹介しましたが、コミット間を移動するコマンドと cherry-pick コマンドを自動的に行ってくれるため遥かに楽な方法が rebase -i の「対話的リベース」です。

裏で行われる書き換えのイメージは、チェリーピックで紹介した形と同じです。

対話的リベースの使い方についてはここでは紹介しませんが、詳しく書かれた「7.6 Git のさまざまなツール - 歴史の書き換え」などがありますので是非ご確認ください。  

履歴の書き換え時に役立つコツ

コミット履歴を整理した後は git diff を使って検算

コミットの削除、分離と統合 (cherry-pick を使った方法)』でも触れましたが、基本的にコミットの並びや、分離/統合を行っただけなら最終的なソースツリーは元と変わらないはずです。この旨を履歴の変更後に git diff 元のコミットID..新コミットID で確認すると安心できます。

rebase -i を使った場合は元のコミットに ORIG_HEAD という目印が作られので git diff ORIG_HEAD.. で確認できます。  

二つ以上前のコミットを修正

HEAD にあるコミットは commit --amend で書き換えられますが、HEAD より前のコミットは commit --fixup を使って直すのが便利です。

流れとしては、

  1. commit --amend を行う時のようにステージングまで準備する
  2. コミットは git commit --fixup=&lt;直すコミットのID> で行う
  3. すると "fixup! ○○○○○" というコミットが作られます
  4. --autosquash オプションを指定して対話的リベースを行う
  5. すると、対話的リベースの画面で "fixup!" コミットは自動的に直すコミットに統合されるように配置されます

--autosquash は便利なので、常に rebase -i に指定されるように設定するのがお勧めです。

git config --global rebase.autosquash true

設定後、稀に無効にしたい場合がありますが、その際は rebase -i --no-autosquash で対話的リベースを行います。  

見やすい git log

git log--graph--decorate 指定したほうがブランチの位置関係が追いやすくなります。

私は以下のエイリアスを定義して通常は git lg と打つようにしています。

git config --global alias.lg 'log --graph --decorate'

 

もっと分かりやすい競合マーカー

履歴の書き換えを行っていると競合もそれなりに伴います。最終結果を同じにしたい場合でも、中間コミットの並び変えによって中間コミットの diff が変わってくるからです。

git config --global merge.conflictstyle diff3

この設定は、競合時の &lt;&lt;&lt;&lt;&lt;&lt;&lt; >>>>>>>> のマーカーを次のように見やすくしてくれます。

設定なしの競合マーカー。

<<<<<<<
我々が入れたコード
=======
競合した相手が入れたコード
>>>>>>>>

設定有の競合マーカー。

<<<<<<<
我々が入れたコード
|||||||
どちらの改修も入る前の状態
=======
競合した相手が入れたコード
>>>>>>>>

 

競合を押し倒せる時がある

以下の状態から AB を逆にしようとします。

  ----> X --> A --> B
        ↑           ↑
  origin/master    foo

求めている結果はこのような履歴です。

                     foo
                      ↓
          .--> B' --> A'
         /
  ----> X --> A --> B
        ↑
  origin/master

しかし、X --> B' を作る際に競合が発生しました。仕方ないので &lt;&lt;&lt;&lt;&lt;&lt;&lt; >>>>>>>> の間を修正します。いざ進むと次は B' --> A' で競合が発生しました。(並びの交換はシンメトリー上片方で競合するとほぼ確実にもう片方でも競合します。)

ただ、B' --> A' の時は &lt;&lt;&lt;&lt;&lt;&lt;&lt; >>>>>>>> の間を直す必要がありません!

元の BA' のソースツリーが同じになることが分かっているからです。

なので、競合マーカーは一気に無くせます。

# ワーキングツリーに B の状態を反映
git checkout B .

[ポイント] 第三引数を指定有りの git checkout &lt;commit> &lt;path> では、&lt;commit> の状態にしたいファイルを &lt;path> に指定します。. は現フォルダなので、上のコマンドでは配下全てのファイルが B の状態になります。  

一度スタッシュしてからバグフィックスを作る

A君は改修中に過去のバグを見つけた時に、バグフィックスを他の改修と混ぜたコミットにしため後から分離する必要が生じました。

3. wip: ちょっと設計を変更、それと過去のバグを見つけたのでついでに修正

コミットを作る時に行単位でバグフィックスのみステージングに入れてバグフィックス単体のコミットを作る方法もありますが、バグフィックスの範囲が広いときは、一度現行の改修を保留して、先にバグを直した方が後々伸びた履歴の並び変え時の競合を避けられるかもしれません。

現行の改修を保留してバグフィックスを行う手順は、

  1. やりかけの改修を git stash 1 で一旦保留
  2. 次に origin/master に移動してバグフィックス用のブランチを作成
    • 例: git checkout -b fix_some_bug origin/master
  3. バグフィックスを行ってコミット
  4. 従来の修正をこのバグフィックス用のブランチの後にリベース
    • 例: git rebase fix_some_bug feature_foo

1 git stash を使うよりも git commit -a -m wip などで wip コミットを作るのを好む人もいます。stash を使うのに比べて明確にブランチごとに管理できることが特徴です。(この場合は git stash pop の代わりに git reset HEAD~ で戻します。)

最後に

綺麗なコミット履歴の恩恵は、レビュアーだけにもたらされるわけではありません。

私は、改修をレビューに出す前に整列した履歴を一コミットづつ diff を読み直すようにしていますが、このタイミングでバグが見つかったり、考えていた機能の実装漏れに気づけることがあります。

見落としていたバグを一つでも見つけられれば、履歴を綺麗にする時間の意味もあるかと思っています。 また、過去のコミット履歴を確認する際にも、目的が明確な履歴が残っていることで理解がしやすいです。

一見、時間は掛かりますが、それを上回る利点があると考えています。

私は、Git が可能にしたこの手法で、システムに与えるバグの件数が減らすことができました。(大切なことなので2回言いました(笑))

マネーフォワードでは、コミット履歴も大事にするエンジニアを絶賛募集しています。 ご応募お待ちしています。

【採用サイト】 ■『マネーフォワード採用サイト』 https://recruit.moneyforward.com/『Wantedly』 https://www.wantedly.com/companies/moneyforward

【公開カレンダー】 ■マネーフォワード公開カレンダー

【プロダクト一覧】 ■家計簿アプリ・クラウド家計簿ソフト『マネーフォワード』 https://moneyforward.com/家計簿アプリ・クラウド家計簿ソフト『マネーフォワード』 iPhone,iPad家計簿アプリ・クラウド家計簿ソフト『マネーフォワード』 Androidクラウド型会計ソフト『MFクラウド会計』 https://biz.moneyforward.com/クラウド型請求書管理ソフト『MFクラウド請求書』 https://invoice.moneyforward.com/クラウド型給与計算ソフト『MFクラウド給与』 https://payroll.moneyforward.com/消込ソフト・システム『MFクラウド消込』 https://biz.moneyforward.com/reconciliation/マイナンバー対応『MFクラウドマイナンバー』 https://biz.moneyforward.com/mynumber