Clean git

Posted:

Well-organized commits are a treat. They are your guide to the theory of developers come and gone, a ledger filled with important decisions. Knowing how to structure commits into a readable, retrievable, and reviewable history is one of the keys to a maintainable codebase.

Compare a change request (CR) with the following commits:

8e74f78 Add new button feature WIP

47e9c34 Fix linter

b409804 Finish button feature

0643984 Cleanup

To one with more detail, yet fewer commits:

8e74f78 feat: Add disabled state to button

47e9c34 fix: Button tooltip and disabled state clash

When a button has a tooltip and is disabled, ensure that the tooltip
theme matches that of the disabled state.

Before even looking at the code, which CR would you prefer reviewing?

This isn't a lesson on how to write good commits, because cbeams has that down to a science. Instead, this post covers the tools and workflows that make writing good commits easier.

Responding to CR comments

There are few experiences more frustrating than using git blame on a line of code, praying some amount of insight can save your lack of understanding, only to be met with "PR changes".

def add_ten(v)
  # git blame: You, 3 years ago. "PR changes"
  if something_that_has_nothing_to_do_with_anything
    v + 15
  else
    v + 10
  end
end

Please do not create, let alone merge, commits titled "PR changes". Instead,

Fixing commits

The temptation to create the "PR changes" commit comes from a good place. You don't want to force push your branch, because it makes it hard to re-review changes and breaks local collaborators. Moreover, those commits aren't really doing anything new—they're just fixing what you wrote previously.

Fixups offer a better way. Say you have the following commits in CR:

$ git log --oneline
b409804 feat: Disable form on validation
8e74f78 feat: Add disabled state to button
22e8539 Initial commit

Your CR reviewer points out a bug introduced by your button component, so you make the changes and stage them:

$ git status -s
M Button.tsx

Your new changes are necessary to fix your prior commit, "feat: Add disabled state to button", and don't belong as a separate thought. Instead of creating a new commit manually, use git commit --fixup <SHA>, indicating that your changes fix a prior commit:

$ git commit --fixup 8e74f78
[main e69dff9] fixup! feat: Add disabled state to button
 1 file changed, 1 insertion(+), 1 deletion(-)

$ git log --oneline
e69dff9 fixup! feat: Add disabled state to button
b409804 feat: Disable form on validation
8e74f78 feat: Add disabled state to button
22e8539 Initial commit

With --fixup, git automatically creates a new commit for you with a special fixup! prefix. The rest of the commit's title is identical to the one it's fixing.

Now you're ready to git push that branch and await your CR approval.

Squashing fixups for clean history

Fixups are great for CRs, but you don't want them to clutter your project's history. After CR approval, you need to perform an extra step before you can merge your CR into the main branch.

Use interactive rebase with the --autosquash option to automatically squash your fixup commits into their respective targets:

$ git log --oneline
e69dff9 fixup! feat: Add disabled state to button
b409804 feat: Disable form on validation
8e74f78 feat: Add disabled state to button
22e8539 Initial commit

$ git rebase --interactive 22e8539 --autosquash

Within the interactive rebase window, your fixup! commits are automatically repositioned with the fixup action (instead of pick):

pick 8e74f78 feat: Add disabled state to button
fixup e69dff9 fixup! feat: Add disabled state to button
pick b409804 feat: Disable form on validation

Once you save this window your fixup commits are squashed into their target commits and your history is left clean:

$ git log --oneline
b409804 feat: Disable form on validation
8e74f78 feat: Add disabled state to button
22e8539 Initial commit

Now you can git push --force this branch and merge it into main.

Unwinding over-eager commits

Sometimes you're in a rush to get your changes up on the server, so you throw all of your changes into one big "WIP" commit:

$ git commit -am "WIP"

That's fine for local development, but as soon as you expect someone to review your CR you had better break apart your history into something meaningful.

A common strategy is to reset your changes and commit the files one-by-one:

$ git reset HEAD~1

$ git status -s
M Button.tsx
M Form.tsx
?? helper.ts

$ git add Button.tsx
$ git commit -m "feat: Add disabled state ..."

This strategy works great when every change to a file represents the same thought. But what about when you make two very different changes to the same file?

As a contrived example, say you have two new additions to a Rails controller:

$ git diff
@@ -1,3 +1,9 @@
 class PostsController < ApplicationController
+  def index
+    @posts = Post.all
+  end

+  def new
+    @post = Post.new
+  end
 end

Adding this file in one go makes it harder to describe the commit since you're changing two different and unrelated pieces of code.

Rather than staging the entire file, you want to stage each method separately. To do this, pass in the --patch option when adding the file. This enters interactive patch mode, which allows you to add your changes as separate units. Each unit is referred to as a hunk.

$ git add --patch
@@ -1,3 +1,9 @@
 class PostsController < ApplicationController
+  def index
+    @posts = Post.all
+  end

+  def new
+    @post = Post.new
+  end
 end
(1/1) Stage this hunk [y,n,q,a,d,s,e,?]?

In interactive mode, you're brought to a screen that looks very similar to a normal git diff, with the addition of a "Stage this hunk" question at the bottom.

You don't want to stage this hunk as-is. Instead, you want to split it apart (s):

(1/1) Stage this hunk [y,n,q,a,d,s,e,?]? s
Split into 2 hunks.
@@ -1,2 +1,5 @@
 class PostsController < ApplicationController
+  def index
+    @posts = Post.all
+  end

(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?

When you split a hunk, git will automatically divide the changes into two smaller pieces. After splitting, you can see that the preview window shows (1/2) instead of (1/1), indicating that you have two separate hunks ready to stage.

Enter yes (y) on the first hunk and no (n) on the second, staging the changes made to "index" but not the changes made to "new":

(1/1) Stage this hunk [y,n,q,a,d,s,e,?]? s
Split into 2 hunks.
@@ -1,2 +1,5 @@
 class PostsController < ApplicationController
+  def index
+    @posts = Post.all
+  end

(1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? y
@@ -2,2 +5,5 @@

+  def new
+    @post = Post.new
+  end
 end
(2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? n

Take a look at your git status. You'll see that your original file has both unstaged and staged changes, since you accepted the "index" hunk and declined the "new" hunk:

$ git status
Changes to be committed:
modified: app/controllers/posts_controller.rb

Changes not staged for commit:
modified: app/controllers/posts_controller.rb

Now you can create two commits, one for each method:

$ git commit -m "feat: Add index"
$ git add app/controllers/posts_controller.rb
$ git commit -m "feat: Add new"

Review

Commits are important for maintaining the health of your codebase and the sanity of your CR reviewers. Remember these key tools:


Thanks for reading! Send your comments to [email protected].