New convention for commit messages on GitHub?


(Niels Dekker) #1

According to ITK’s commit-msg hook, “The first line may be at most 78 characters”. Unfortunately, when the first line of a commit message has more than 72 characters, it gets chopped off, replacing the chopped off characters by “…”, as it appears at https://github.com/InsightSoftwareConsortium/ITK/commits :roll_eyes:

Should the commit-msg hook be modified to only allow up to 72 characters? Or could the GitHub installation of ITK be adjusted to show up to 78 characters, without chopping or “…”?

I’m in favor of allowing clear and explicit commit messages, so if possible, adjusting the GitHub installation would be my preference! :grinning:


(Dženan Zukić) #2

I don’t think GitHub instance can be adjusted - I just took a look at settings and could not find anything. I guess somebody will have to update the commit message hook.


(Niels Dekker) #3

Thanks, @dzenan You see, the first line of the commit message of my proposed patch https://github.com/InsightSoftwareConsortium/ITK/pull/204/commits/9e94b2b5cdd0563f40b621d63c84186dd52f6d23 has 72 characters, which appears “GitHub friendly”:
BUG: ShapedImageNeighborhoodRange should not try to avoid rvalue offsets

But it chopped off this one of yours, which has just one more character:
Merge remote-tracking branch 'remotes/upstream/release-4.13' into release
at https://github.com/InsightSoftwareConsortium/ITK/commit/d178c9f5cc391650adc1d083311d18fc083d7bc7


(Dženan Zukić) #4

I just looked at https://github.com/InsightSoftwareConsortium/ITK/tree/master/Utilities/Hooks and could not see where line limit of 78 characters is checked. I did notice an unrelated out of date message.


(Matt McCormick) #5

The client side Git hook is in the commit-msg file on the hooks branch of the ITK repository.

There is also a check in the ghostflow status check – adding more detailed information on what this does and a reference to its configuration is a work in progress.

Ideally, we could extend the GitHub supported line length, but I do think that is possible. There is a tradeoff between enough characters to make a meaningful commit summary and conciseness for tools that use that summary. From my perspective, it is already difficult enough to hit the 78 character limit, and 72 is too short.

CC: @brad.king


(Dženan Zukić) #6

@matt.mccormick Is the stuff in Utilities/Hooks used any more? If not, we could remove it. And if we don’t remove it we should update it by copying stuff in the hooks branch.


(Matt McCormick) #7

Both are used. The hooks branch contains scripts shared across many projects, and it should not be copied / forked because that would be difficult to maintain. The Utilities/Hooks hooks contains additional ITK-specific hooks that are also executed after the vanilla hooks.


(Brad King) #8

The ghostflow check can be updated from 78 to 72 with a one-line change to the deployment’s configuration. @matt.mccormick it sounds like you don’t want to make it shorter though.


(Niels Dekker) #9

Honestly I also find it hard to always keep the first line <= 72 chars. So maybe it would be too harsh to reject any commit message that has first line > 72.

Maybe instead, what about just having a gentle suggestion for contributors, just to try to keep that first line <= 72 chars…? Maybe as a “loose” (not a strict) style guideline?


(Brad King) #10

what about just having a gentle suggestion for contributors

Does GitHub’s “…” truncation already provide that?


(Niels Dekker) #11

Once GitHub has truncated the line, it’s too late already! It took me a while to figure out that it always happens when the line > 72 chars, and never when <= 72 chars. So I think it could be helpful that contributors get informed that exceeding 72 chars will lead to truncation at GitHub.


(Matt McCormick) #12

How about suggesting that commit message subjects should be less than 72 characters, if possible, in the prepare-commit-msg hook?


(Brad King) #13

Suggesting it in prepare-commit-msg sounds good.

How useful have the BUG:, ENH:, etc. prefixes been in practice? They take up 5-6 characters making the limit harder to meet.


(Matt McCormick) #14

These are very useful for scanning the commit history.


(Brad King) #15

What workflow/use-case involves scanning history in a way that such prefixes are useful?


(Matt McCormick) #16

Any time you are browsing the history and looking for changes of a given type, or want to understand what is the nature of changes without needing to fulling read and process the commit message. I use it all the time.


(Niels Dekker) #17

The BUG/ENH/STYLE/... prefix requirement also helps me as I’m trying to follow the principle of “One Commit One Change”, when possible. Specifically I think it’s a good practice to have separate commits for a bug fix, an enhancement and a style improvement. The required prefix supports this practice, in my opinion.

My 2 cents, Niels


(Matt McCormick) #18

A pull request to suggest a summary of 72 characters or less is here:


(Dženan Zukić) #19

If GitHub cannot be adjusted to not truncate commit summaries of lengths 73 to 78 characters, I am in favor of imposing the limit of 72. I am afraid that just suggesting 72 will not accomplish much.


(Matt McCormick) #20

If the summary is longer that 72 characters, the remainder of the message shows up as in the commit summary, which is not a show-stopper.