May 31, 2022

By Cayle Sharrock

Git farming

or, How to get your PRs merged into Tari

When contributing new code to open source projects, communicating why the change should be made is almost as important as the code itself. The bigger the PR, the truer this becomes.

In this post, I’ll explain what git commit farming is, and how it will help you get your PRs merged into Tari faster, and with less fuss.

P2P chess!

WTF are git logs?

Tari is hosted on Github, which started out as a nice web-based front-end for git. Things changed over time and today Github does a lot more than proxy git pulls and pushes. But deep down at the core, git is still there, doing the grunt work behind Github.

Git is a really complex tool. I doubt I know 25% of its full capability. So this post will focus on one particular aspect of git that is really valuable in large open-source project management: farming git logs.

Hello logs

If you know what git logs are and how to read them, skip ahead to the next section. There are more goodies for you down there, I promise.

Git logs are the messages that get written every time a commit is made in a repository. You make a commit when running

  • git commit - your editor will open and you’ll be asked to write a message
  • git merge - git will write the commit message itself. And for this reason, rebases are often preferred over merge commits.
  • Github PR descriptions - when PRs are merged, Github will use the PR description as the commit message (possibly throwing away intermediate commit messages that the author has provided - more on that later).

To read the commit log, simply run this command from a terminal:

$ git log

Here’s a snippet from the tari repo’s logs:

commit 2636cb56ddf1b67ed7bf4c4aea8c05f9369b11d0 (HEAD -> development, origin/development, origin/HEAD)
Author: mrnaveira <[email protected]>
Date:   Mon May 30 14:54:40 2022 +0100

    feat(core): add contract acceptance utxo features (#4145)
    
    Description
    ---
    - Adds `ContractAcceptance` struct to the side-chain output features
    - Implements consensus encoding/decoding for `ContractAcceptance`
    - Updates related gRPC types and conversions
    
    Motivation and Context
    ---
    Create structs for contract acceptance in the side-chain features.
    
    How Has This Been Tested?
    ---
    - New unit test for consensus encoding/decoding of the `ContractAcceptance`
    - Existing unit/integration test pass

commit 2b1a69a9219f29472d6fa26b1b2350be7880b11a
Author: stringhandler <[email protected]>
Date:   Mon May 30 15:41:10 2022 +0200

    fix: move peer dbs into sub folders (#4147)
    
    Description
    ---
    Add sub folders for each application's peer DB
    
    Motivation and Context
    ---
    If you run the base node and validator node in the same folder, you will get a file lock error. I added the same folder to the console wallet to keep it the same
    
    How Has This Been Tested?
    ---
    cargo test

commit f79d383533c2e9c4db95d4a13973992c9a8739ef
Author: Denis Kolodin <[email protected]>
Date:   Mon May 30 15:35:28 2022 +0300

    feat: add encrypted_value to the UnblindedOutput (#4142)
    
    Description
    ---
    The PR adds extra encrypted field to `UnblindedOutput` structs.
    
    What was done:
    - Added `EncryptedValue` struct
    - Implemented `ConsensusEncode`, `ConsensusDecode` and `ByteArray` for it
    - Added the `encrypted_value` field of that type to the `UnblindedOutput` struct
    - Added an extra parameter to metadata signature calculation methods
    - Added an extra field to the `GRPC` protocol (`UnblindedOutput` type)
    - Added an extra column and migrations for the `outputs` table
    - The types `OutputSql` and `NewOutputSql` updated to store and restore the `EncryptedValue`
    
    Important notes:
    1. The value of `encrypted_value` passed as a parameter to the metadata signature evaluation but has not taken into account yet, because it will break the verification process of the `TransactionOutput` and we can complete that when the same `encrypted_value` field will be added to the `TransactionOutput` struct in the further PRs.
    2. Real encryption process is not implemented yet. Instead of that I've added the method `todo_encrypt_from` that I used to mark the places where we have to put an encryption service and call it.
    
    Motivation and Context
    ---
    Add an encrypted amount to the `TransactionOutput`.
    
    How Has This Been Tested?
    ---
    CI

commit 5cc9a406a807cfaeaba4e71e2af13fa3b4bde2ff
Merge: 3656c3247 987972cff
Author: Mike the Tike <[email protected]>
Date:   Mon May 30 12:51:59 2022 +0200

    Merge branch 'testnet-dibbler' into development
    
    # Conflicts:
    #       base_layer/wallet_ffi/src/lib.rs
    #       base_layer/wallet_ffi/wallet.h

You will notice a couple of things:

  1. The author, commit hash and timestamp are all contained in the messages.
  2. You can get a clear sense of what the commit contributed. In this snippet, the descriptions all come from Github PR descriptions (because they use the PULL_REQUEST template the project has set up).
  3. The last commit is a merge-commit, where changes from the testnet-dibbler branch were pulled into development.

Log summaries

Sometimes, you don’t want such detailed log data. It turns out that there’s an enormous amount of flexibility in how logs are displayed. The git log command has, per git’s modus operandi, a gazillion tweaks and options.

A commonly-used one is to just print a one-line summary of each commit:

$ git log --oneline

2636cb56d (HEAD -> development, origin/development, origin/HEAD) feat(core): add contract acceptance utxo features (#4145)
2b1a69a92 fix: move peer dbs into sub folders (#4147)
f79d38353 feat: add encrypted_value to the UnblindedOutput (#4142)
5cc9a406a Merge branch 'testnet-dibbler' into development
3656c3247 ci: docker image build for x86-64 & arm64 from x86-64 (#4135)
310a2d202 feat: scan base node for constitutions (#4144)
b4991a471 (leet/development) feat(wallet): new command to publish a contract definition transaction (#4133)
52ecb4940 chore(deps): bump async from 2.6.3 to 2.6.4 in /applications/tari_collectibles/web-app (#4143)
71c3a8a8f test: coverage for rolling average time struct (#4116)
e5d20f7ad test: coverage mocks for files common configuration files (PR #4116) (#4130)
d1827ea41 chore: update croaring and randomx (#4136)
ada31432e feat(core)!: add side-chain features and constitution to UTXOs (#4134)
446406491 fix(integration_test): fix wallet-cli integration tests (#4132)
682aa5d0e feat(core): impl consensus encoding for bool (#4120)

You can log the set of commits that would be included in a merge of two commits are branches by running

$ git log commitA...commitB.

Let’s consider a dummy repo with the following commit history:

(main): A -> B -> C -> D
(branch)      \-> B' -> C'

$ git log --oneline main
4888619 (HEAD -> main) D
7e709a9 C
a123a17 B
bd48e7f A

$ git --oneline branch
3c0915c (branch) C'
5e5ce61 B'
a123a17 B
bd48e7f A

Since we branched off at B, the commits that would be involved in a branch are C, D, B', C' which is what git log --oneline main...branch gives us:

$ git log --oneline main...branch 
3c0915c (branch) C'
5e5ce61 B'
4888619 (HEAD -> main) D
7e709a9 C

Note: The order doesn’t matter. branch...main gives the same result as main...branch

There are lots of other ways to use git log. Read the docs and try some of them out!

PR descriptions vs. Git logs

Depending on how PRs are merged in Github, the git messages get thrown away in favour of the PR description. Since we treat the git commit messages as very important, we retain them and add them to the end of the PR description when merging PRs from Github.

Producing great git commit messages

The git log is a really important resource in understanding not only how a code base works, but understanding its history and gaining an appreciation for the reasoning behind why changes were made.

So how do we produce great git message logs when it’s impossible to anticipate how coding is going to progress when writing a new feature? You always end up with dozens of little commits that are just tiny bugfixes, formatting issues and any number of things that deviate from the platonic description of the code base’s evolution.

The good news is that this is fine. Really.

really

Starting with a plan

Let’s assume we’re writing a new feature in our codebase. As usual, we start from a new branch:

(main)$ git checkout -b my_feature
Switched to a new branch 'my_feature'

We begin by writing a stub API. After writing this, I make a commit:

$ git log
commit dfa1ca73867ed36ad3f7638b71e95ed23329499f (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:10:59 2022 +0100

    Add sketelal API for Feature
    
    No tests or code yest, just the
    * `add_data` function
    * `process_data` function

Off to the races! Ok, now to add some tests…

commit c05ad1a1a8f456fa398ee73bfc03ae1e580900b8 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:13:15 2022 +0100

    Add import tests
    
    * add_data returns Ok(n) returning the number of records when adding
      data
    * `process_data` returns Ok() on succesful processing
    
    Both these tests currently fail

Cool. Now to implement the body of the API so that the tests pass.

Hmm. This is taking longer than anticipated. There’s some issue on the backend that I can’t fix right now. So let’s commit what we’ve done and continue tomorrow..

commit 563ee327276ba199b66053bb668cf60b1c49b0a5 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:16:33 2022 +0100

    WIP Implement add_data
    
    add_data ingests data into the store and returns the total number of
    records.
    
    WIP Backend times out sometimes. Investigating

Fresh day, fresh issues. We’ve identified the problem and fixed it on the backend.

commit 7323efc6db54d63efc2ff0f5b67f2ff1677c3b73 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:30:36 2022 +0100

    Fix timeout issues on backend
    
    The backend expected inserts to be done in negative time.
    This fix sets a reasonable timeout of 500ms

Cherry-picking to submit an independent PR

This bugfix should actually go in as a PR on main, so let’s create a PR now quickly by cherry-picking this commit:

$ git checkout main
Switched to branch 'main'

 (main)$ git checkout -b timeout-fix
Switched to a new branch 'timeout-fix'

 (timeout-fix) $ git cherry-pick 7323efc6db54d63efc2ff0f5b67f2ff1677c3b73
[timeout-fix 6e9e937] Fix timeout issues on backend
 Date: Wed Jun 1 13:30:36 2022 +0100
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 backend_fix.rs

 (timeout-fix) $ git log
commit 6e9e93784c55f056f813978b0673ecc8edd3aedc (HEAD -> timeout-fix)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:30:36 2022 +0100

    Fix timeout issues on backend
    
    The backend expected inserts to be done in negative time.
    This fix sets a reasonable timeout of 500ms

commit 4888619a51c7ce128dbe5f6a60b55b87cd152363 (main)
Author: CjS77 <[email protected]>
Date:   Tue May 31 15:18:45 2022 +0100

    D

Awesome! Our fix can now be pushed as a standalone PR!

It gets reviewed and merged pretty quickly. So I can pull from the remote server and see that the main branch now includes this fix:

(main)$ git log --oneline 
6e9e937 (HEAD -> main, timeout-fix) Fix timeout issues on backend
4888619 D
7e709a9 C
a123a17 B
bd48e7f A

Tidy up the obsolete branch

Great! Time to nuke that timeout-fix branch (it’s no longer needed) and jump back into my-feature:

(main) $ git branch -d timeout-fix 
Deleted branch timeout-fix (was 6e9e937).

 (main) $ git checkout my_feature 
Switched to branch 'my_feature'

Continue with the feature development

After a few hours, I’ve fixed the implementation and committed the changes:

$ git log
commit e668f3f5b2ec11a137fc84c8936e50360d476b21 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:47:36 2022 +0100

    Complete data ingestion
    
    Data ingestion works. Increasing the timeout allows the backend to
    respond, and the add_data test now passes.

Things start getting messy

Almost ready for a PR.. but wait! Let’s run cargo clippy first, since it’s a CI requirement that clippy gives the green light, so best to cut off any issues here.

Shoot! There are some clippy lints. No worries. That’s a quick fix…

$ git log
commit 623e3573e1160c1791798c969e1bfe00acc37b67 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:50:24 2022 +0100

    Clippy fixes
    
    Functional programming FtW

Crumbs. While I was waiting for tests to run I realised that I should also add a delete_record method to the API, along with some tests of course.

Ok, let’s handle that with the next set of commits:

$ git log --oneline 
4bfe58e (HEAD -> my_feature) Implement `delete_record`
64408c3 Add test for `delete_record`
3a21ec7 Add `delete_record_ method to API
623e357 Clippy fixes
...

Ok, tests pass. Time to create a PR.

But as I’m about to submit the PR, something occurs to me: An edge case on add_record. This leads me a bit down a rabbit hole and my git log is becoming increasingly messy:

$ git log -n 6
commit f73547597e41b3b3ea0b7c52b3a883f390e13b88 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 14:48:31 2022 +0100

    Feature complete.
    
    Tests pass
    Clippy passes
    Linting done

commit 0c54146fbe190106d0687c0d30ed39101142b204
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 14:47:57 2022 +0100

    fixup! Stupid off by one error
    
    We'll get it this time. It's 1am after all

commit b2985fc36f6534fc468c8d9c53cb9799c638eb20
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 14:47:19 2022 +0100

    Fix error in process_data
    
    The null test case broke `process_data`. Adding a fix.

commit 6f947fb3031bb15ba624a16b764cc626dcaa68cb
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 14:46:22 2022 +0100

    Add guard for empty record in `add_data`
    
    Catch the case where empty records are provided in `add_data` and return
    an error.

commit d328a81c1e08b86934abd240a32a42be3e13d306
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 14:45:08 2022 +0100

    Add edge_case test for `add_data`
    
    If I add an empty record, the backend will crash! We must guard against
    this.
    
    Test currently fails.

commit 4bfe58e198dc45c448035e5035bc43bfab06c1dc
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 14:41:47 2022 +0100

    Implement `delete_record`

Taking stock, and figuring out what we want

We’re ready to submit a new PR, but my git log is a disaster! No-one reviewing this will have any idea what the thinking behind the feature is beyond the fact that it’s a lot of caffeine-driven and sleep-deprived drivel.

Here’s what the current log summary looks like. I’ve gone and characterised each commit and noticed that there are really 4 main categories of commit message -

  • API definition,
  • tests,
  • API implementation,
  • and formatting/debugging:
$ glg
f735475 (HEAD -> my_feature) Feature complete.   <------ minor 
0c54146 fixup! Stupid off by one error           <------ minor 
b2985fc Fix error in process_data                <------ minor 
6f947fb Add guard for empty record in `add_data` <------ API impl 
d328a81 Add edge_case test for `add_data`        <------ Tests
4bfe58e Implement `delete_record`                <------ API impl 
64408c3 Add test for `delete_record`             <------ Tests
3a21ec7 Add `delete_record_ method to API        <------ API def
623e357 Clippy fixes                             <------ minor
e668f3f Complete data ingestion                  <------ API impl
7323efc Fix timeout issues on backend            <------ Bug fix
563ee32 WIP Implement add_data                   <------ API impl 
c05ad1a Add import tests                         <------ Tests
dfa1ca7 Add sketelal API for Feature             <------ API def
4888619 D                                        <------ branched off main

What we need to do is clean up the git history and tell a unified story. In particular, we want the following:

  1. We really only want 3 commits here: API definition, then tests, then implementation. That will give reviewers a clear idea of what the feature should do, and how it’s done.
  2. The formatting / bugfix commits should get bundled in with of the 3 main commits that’s most relevant to the changes.
  3. The timeout bugfix commit should disappear because it’s already in main.

In the meantime, other PRs have been merged, so main is quite a few commits down the line from us now. We’d also like our PR to look like we’ve branched our PR from the latest HEAD of main (F), rather than the one from a few days ago (D).

(main) $ git log --oneline
480454e (HEAD -> main) F             <--- We want to have branched from here
feb4b05 E
6e9e937 Fix timeout issues on backend
4888619 D                            <--- We branched from here
7e709a9 C
a123a17 B
bd48e7f A

git rebase (interactive) to the rescue

It turns out we can do all of this with one git command:

git rebase --interactive main

What this command says is “take all the commits I have made since deviating from main (D) and move (or re-base, if you will) the entire branch onto the head of main branch. Oh, and let me edit all the commit messages first!”

Ok, let’s see what happens when we run this magical command. It actually opens up an editor (vim, in my case) and gives me this meta-commit file to edit:

pick dfa1ca7 Add sketelal API for Feature
pick c05ad1a Add import tests
pick 563ee32 WIP Implement add_data
pick e668f3f Complete data ingestion
pick 623e357 Clippy fixes
pick 3a21ec7 Add `delete_record_ method to API
pick 64408c3 Add test for `delete_record`
pick 4bfe58e Implement `delete_record`
pick d328a81 Add edge_case test for `add_data`
pick 6f947fb Add guard for empty record in `add_data`
pick b2985fc Fix error in process_data
pick 0c54146 fixup! Stupid off by one error
pick f735475 Feature complete.

# Rebase 480454e..f735475 onto 480454e (13 commands)
#
# Commands:
# p, pick <commit> = use commit
# r, reword <commit> = use commit, but edit the commit message
# e, edit <commit> = use commit, but stop for amending
# s, squash <commit> = use commit, but meld into previous commit
# f, fixup <commit> = like "squash", but discard this commit's log message
# x, exec <command> = run command (the rest of the line) using shell
# b, break = stop here (continue rebase later with 'git rebase --continue')
# d, drop <commit> = remove commit
# l, label <label> = label current HEAD with a name
# t, reset <label> = reset HEAD to a label
# m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
# .       create a merge commit using the original merge commit's
# .       message (or the oneline, if no original merge commit was
# .       specified). Use -c <commit> to reword the commit message.
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#

What we have here is

  • Our list of commits, in reverse chronological order.
  • The ability to (pick) leave commits as they are.
  • The ability to (reword) edit commit messages.
  • The ability to combine (squash) commits
  • The ability to reorder commits. (to whit, These lines can be re-ordered; they are executed from top to bottom.)
  • The ability to remove commits (drop)
  • Or shuffle commits under the rug with the previous commit (fixup).
  • And check this: the cherry-picked commit (6e9e937) is not even in this list. Git has noticed that the commit is already in main and has done task number 3 for us already!

This is exactly what we need! Ok, after a few seconds of pruning, grafting and weeding (this is why I call it git farming), I have (leaving out the commented instructions this time):

reword dfa1ca7 Add sketelal API for Feature
squash 3a21ec7 Add `delete_record_ method to API
pick c05ad1a Add import tests
squash 64408c3 Add test for `delete_record`
squash d328a81 Add edge_case test for `add_data`
pick 563ee32 WIP Implement add_data
squash e668f3f Complete data ingestion
fixup 623e357 Clippy fixes
squash 4bfe58e Implement `delete_record`
squash 6f947fb Add guard for empty record in `add_data`
fixup b2985fc Fix error in process_data
fixup 0c54146 fixup! Stupid off by one error
fixup f735475 Feature complete.

You’ll notice:

  • Several commits have been re-ordered. I’ve grouped the API definition, test, and implementation commits together.
  • I’ve also squashed or fixup’d all related commits, keeping commit messages that will be helpful and discarding the useless ones.
  • I also want to fix that typo in dfa1ca7, and so will reword that message.

Save, quit and let’s go!

Rewording a message

The first message I need to edit is commit number dfa1ca7. All I want to do here is fix the typo in the heading (sketelal -> skeletal) and text (yest -> yet):

Add skeletal API for Feature

No tests or code yet, just the
* `add_data` function
* `process_data` function

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Date:      Wed Jun 1 13:10:59 2022 +0100
#
# interactive rebase in progress; onto 480454e
# Last command done (1 command done):
#    reword dfa1ca7 Add sketelal API for Feature
# Next commands to do (12 remaining commands):
#    squash 3a21ec7 Add `delete_record_ method to API
#    pick c05ad1a Add import tests
# You are currently editing a commit while rebasing branch 'my_feature' on '480454e'.
#
# Changes to be committed:
#       new file:   api.rs
#

Once I save the file and quit vim, the rebase command moves to the next commit in the list, squash 3a21ec7.

Squash gems

Once again, I’m thrown into vim, and presented with a new commit message. Looking carefully, you’ll notice that it’s the message I’ve just edited, and the message from when I added the delete_record function.

# This is a combination of 2 commits.
# This is the 1st commit message:

Add skeletal API for Feature

No tests or code yet, just the
* `add_data` function
* `process_data` function

# This is the commit message #2:

Add `delete_record_ method to API
# This is a combination of 2 commits.
# This is the 1st commit message:

Add skeletal API for Feature

No tests or code yet, just the
* `add_data` function
* `process_data` function

# This is the commit message #2:

Add `delete_record_ method to API

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# interactive rebase in progress; onto 480454e
# Last commands done (2 commands done):
#    reword dfa1ca7 Add sketelal API for Feature
#    squash 3a21ec7 Add `delete_record_ method to API
# Next commands to do (11 remaining commands):
#    pick c05ad1a Add import tests
#    squash 64408c3 Add test for `delete_record`
# You are currently rebasing branch 'my_feature' on '480454e'.
#
# Changes to be committed:
#       modified:   api.rs

To make it look like it was my plan all along to define the API with three functions, I’ll edit the file like so:

Add skeletal API for Feature

No tests or code yet, just the
* `add_data` function
* `process_data` function
* `delete_record` function

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# interactive rebase in progress; onto 480454e
# Last commands done (2 commands done):
#    reword dfa1ca7 Add sketelal API for Feature
#    squash 3a21ec7 Add `delete_record_ method to API
# Next commands to do (11 remaining commands):
#    pick c05ad1a Add import tests
#    squash 64408c3 Add test for `delete_record`
# You are currently rebasing branch 'my_feature' on '480454e'.
#
# Changes to be committed:
#       modified:   api.rs

Once again, save the file, and quit vim to continue rebasing.

Dealing with conflicts

While rebasing, you may encounter merge conflicts. This is something that happens quite frequently when rebasing, especially when you’re working on “busy” parts of the code base, or if you’re trying to rebase over many commits.

These are handled the same way you usually resolve merge commits. Some tips:

  • Use the same tools you usually do for handling merge conflicts. I really like IntelliJ’s conflict resolution tools but YMMV.
  • Remember to be careful not to undo other people’s code by accident.
  • In some cases, the conflicts are so thorny that it’s better to abandon the rebase (git rebase --abort) and try clean up your commit message another way. But uh, let this be a last resort and not your go-to strategy. Please.

After you’ve fixed the commits, you can continue the rebase with git rebase --continue

More squashes

I squash all the test-based commits together and make it look like one unified commit:

# This is a combination of 3 commits.
Add import tests

* add_data returns Ok(n) returning the number of records when adding
  data
* `process_data` returns Ok() on successful processing
* `delete_data` returns Ok() on a successful delete
* Calling `add_data` with an empty record returns an error

All these tests currently fail

And do something similar for the implementation:

Implement API

* add_data ingests data into the store and returns the total number of
records. It returns an error if an empty record is given.
* `delete_record` deletes a record. It returns `Ok()` if successful.

Fixups

The last few commits are all fixups. You’ll notice that you still an opportunity to edit the commit message, but the irrelevant comments in the “fixup” commits are already commented out:

# This is a combination of 2 commits.
# This is the 1st commit message:

Implement API

* add_data ingests data into the store and returns the total number of
records. It returns an error if an empty record is given.
* `delete_record` deletes a record. It returns `Ok()` if successful.

# The commit message #2 will be skipped:

# Clippy fixes
#
# Functional programming FtW

There are no changes to make here, so just save and quit.

This time, the rebasing runs to completion:

$ git rebase --continue 
Successfully rebased and updated refs/heads/my_feature.

Basking in our victory

Let’s have a look at the git message history now shall we?

Summary first:

 (my_feature)$ git log --oneline 
263e262 (HEAD -> my_feature) Implement API
06e46db Add import tests
aec77c7 Add skeletal API for Feature
480454e (main) F
feb4b05 E
6e9e937 Fix timeout issues on backend
4888619 D
7e709a9 C
a123a17 B
bd48e7f A

Nice, isn’t it? We’ve branched off from F and not D; And there are just three commits, nicely contextualised, that will make it really easy for reviewers to divide and conquer our code during the PR review process.

Let’s see what those commit messages actually look like:

$ git log -n 4
commit 263e262d68d6d9f98f24c4c7dc3a447f3f295b14 (HEAD -> my_feature)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:16:33 2022 +0100

    Implement API
    
    * add_data ingests data into the store and returns the total number of
    records. It returns an error if an empty record is given.
    * `delete_record` deletes a record. It returns `Ok()` if successful.

commit 06e46dba28bc6baf40cdc002a8cbd49494b64668
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 15:38:56 2022 +0100

    Add import tests
    
    * add_data returns Ok(n) returning the number of records when adding
      data
    * `process_data` returns Ok() on successful processing
    * `delete_data` returns Ok() on a successful delete
    * Calling `add_data` with an empty record returns an error
    
    All these tests currently fail

commit aec77c7371f7fb79102005128f2d65f9bd71b0b6
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 13:10:59 2022 +0100

    Add skeletal API for Feature
    
    No tests or code yet, just the
    * `add_data` function
    * `process_data` function
    * `delete_record` function

commit 480454eff9580d50189e37b21b666eec3eeb03b6 (main)
Author: CjS77 <[email protected]>
Date:   Wed Jun 1 15:02:38 2022 +0100

    F

He he. It looks like I’m a coding virtuoso.

What’s this all for?

It clearly takes a fair amount of effort to farm the git commit messages like this. So why bother?

It helps the code reviewers. Simple as that. And the easier we make life for people reviewing our code, the more likely they are to

  • understand our decision-making process, and make reasoned rebuttals rather than knee-jerk ones.
  • Know where to look for bugs
  • Give us that coveted +1 and get us one step closer to having our PR hit production.
Back to all Updates