Proposed commit message format change

Development
1

The project is now big enough that the one liner commit messages aren’t always super unambiguous. Of course, where necessary the body contains more info, but I’d like to propose prefixing the message with the main package name affected to narrow it down. That is, what would currently be the following in a hypothetical “git log”:

* 94b3ce4 The Max{Send,Recv}Kbps variables are supposed to be in KiB/s
* c439c54 messagediff argument order should be expected, actual
* 78120bd Use Bootstrap tooltips instead of plain title attributes
* f66c1c3 Amend norgeous
* 6f82d83 Let "systemctl help" command work
* 17517bc Don't show restart prompt when changing folder label (fixes #2840)
* ba5231d apiService should not reference global variable 'locations' ```

should become

* 94b3ce4 lib/connections: The Max{Send,Recv}Kbps variables are supposed to be in KiB/s
* c439c54 tests: messagediff argument order should be expected, actual
* 78120bd gui: Use Bootstrap tooltips instead of plain title attributes
* f66c1c3 meta: Amend norgeous
* 6f82d83 etc/linux-systemd: Let "systemctl help" command work
* 17517bc lib/model: Don't show restart prompt when changing folder label (fixes #2840)
* ba5231d cmd/syncthing: apiService should not reference global variable 'locations' (hinders testing)

I imagine the “tag” at the front being one of, in order:

  • The main affected package. I.e. lib/connections, lib/model. If a change spans over more of them, this would be the one that is most changed or most relevant. I.e. if we change the API in lib/connections that is the tagged package, even though the change will also result in modifications in cmd/syncthing etc.
  • The changed folder in case it wasn’t a package change, top or maybe second level. etc/linux-systemd and gui are examples there.
  • Something general if it really is a repo-wide change. tests is the example there as this was a replacement that affected tests in many places, and only tests. I’m not sure of the specific guidelines here…
  • Something for changes at the top level. meta for metadata? build for changes to the build system? Just top for everything, regardless? It doesn’t matter super much what we do here I think, but sticking to something is probably a good idea.

What do you think? Is this onerous and painful, does it make sense, etc?

It would fall on us as committers to enforce this on new contributions. Luckily that’s fairly easy as I think the recommended merge strategy is anyway to ask st-review to do the merge and it’s trivial to add the fixed commit message at that point. In the end, people will learn by example. (I’ll of course update the contribution guidelines, but noone reads those anyway.)

2

While I agree in principle, for the sake of a discussion:

  • What if a commit affects more than one package?
  • Why can’t we just run git log with the appropriate subdirectory to get a history for that package?
3

I talk about that in the first bullet point :slight_smile:

Oh, we can. This is more about getting a better overview from the top level git log, than looking for changes to a specific package.

In a way, it’s also a subtle hint that if it’s impossible to find a concise tag to describe a change with, maybe the change should be broken down into smaller changes. :slight_smile:

4

So you do… That’s what I get for reading forum posts while half-asleep on mobile.

As I said, :thumbsup: from me.

5

Note also that this ties in a bit with moving towards a monorepo, as I’ve suggested I might want to do… There’s a relevant clarity difference between

* aaa4159 Add debug performance logging per request

and

* aaa4159 cmd/discosrv: Add debug performance logging per request

in that situation. You might argue that this is just a workaround for the downsides of having too much stuff in the same repo, and I might even agree, but I think that the advantages still outweigh the disadvantages…

6

Although I am not yet a Go programmer (but I seem to like it and I am moving forward), I like the proposal of better commit messages. As a first step, the mergebot has improved the situation a lot by avoiding the force push Github Pullrequest crap.

I have just viewed the current kernel commit log, which has the best commit message standards on earth:

$ git lg --no-merges
...
65b99a0 xfs: refactor unmount record detection into helper      Brian Foster       12 days ago     
82ff6cc xfs: separate log head record discovery from verifica.. Brian Foster       12 days ago     
335e073 klp: remove CONFIG_LIVEPATCH dependency from klp head.. Jiri Kosina        12 days ago     
b24b78a klp: remove superfluous errors in asm/livepatch.h       Miroslav Benes     2 weeks ago     
45f6880 libnvdimm, pmem: fix ia64 build, use PHYS_PFN           Dan Williams       12 days ago     
5d8eb84 cpu/hotplug: Remove redundant state check               Thomas Gleixner    2 weeks ago     
1ae1602 configfs: switch ->default groups to a linked list      Christoph Hellwig  3 weeks ago     
0277e01 spi/rockchip: fix endian mode for 16-bit transfers      Alexander Kochet.. 12 days ago     
7f54ab5 target: Drop incorrect ABORT_TASK put for completed c.. Nicholas Bellinger 12 days ago     
d4f3236 nfit, libnvdimm: clear poison command support           Dan Williams       2 weeks ago     
a921e9b isdn: i4l: move active-isdn drivers to staging          Arnd Bergmann      2 weeks ago     
01ed1e1 isdn: icn: remove a #warning                            Arnd Bergmann      2 weeks ago
...

The prefixed subsystem makes the messages clear; you can even skip particular subsystems easily by inverese grepping.

I would just go with sth like this: README: Fix typo

2 Likes