Hi Tomas, Tomas Volf <[email protected]> writes:
> Maxim Cournoyer <[email protected]> writes: > >> Hi, >> >> Tomas Volf <[email protected]> writes: >> >>> Carlo Zancanaro <[email protected]> writes: >>> >>>> On Thu, May 14 2026, Tomas Volf wrote: >>>>> I have encountered bug with the hook we install for adding Change-Id >>>>> trailer. It for some reason inserts it as a first line of the commit >>>>> message instead of at the end. >> >> Would you mind to share how to reproduce the issue? It isn't obvious >> to me. > > Right, sorry, that was in the initial bug report. Copy&pasting here: > > $ git commit -m '--- test' --allow-empty > [master f07a209fb1] Change-Id: I02675f1053c80563402f1acddd0a64a537f4da55 --- > test > $ git log -1 --format=%B > Change-Id: I02675f1053c80563402f1acddd0a64a537f4da55 > --- test > > Notice that Change-Id line is *before* `--- test' line I see; is there a practical use case for this? This only happens when the message starts with `---`, or when there is a `---` used anywhere in the message? >> >> [...] >> >>>> I'm not sure why we're limiting ourselves to git versions and features >>>> from 2017. >>> >>> Given that the commit-msg hook has this header >>> >>> # From Gerrit Code Review 3.11.1. >>> # >>> # Part of Gerrit Code Review (https://www.gerritcodereview.com/) >>> # >>> # Copyright (C) 2009 The Android Open Source Project >>> >>> I would not rule out the option that Maxim just added it without >>> considering this particular detail. I took the liberty of CC-ing him, >>> maybe he could answer this bit. >> >> It's, as the copyright suggests, a script borrowed from the Gerrit >> project :-). It hasn't been modified. About the ways we call it, if it >> resolves a clear issue, we do not need to constrain ourselves to an old >> git version the same the gerrit project might; using the git version >> which is in our current master tree would be fine, I think. > > Great, since adding the --no-divider seems to do the trick, that could > be a fix. If it doesn't change the normal behavior otherwise, fine by me! > At the same time, since we are just setting a single trailer > to a random value, it might be worth it to simplify the script a lot. I wouldn't want to have to maintain that Gerrit script; they update it sometimes and it's nice to just be able to pull their fixes/updates in our tree. Note that as hinted in GCD 002, its originally intended purpose is not fulfilled, and I'd personally like to do away with it, to simplify things a bit. I think if we want to continue having some stable identifiers for commits we should look into the change-id stored in git commit headers, an effort that Jujutsu, Gerrit and others are pursuing [0]. This as the benefit of keeping a stable identifier available for each commit, without the downside of cluttering the commit message. [0] https://lore.kernel.org/git/[email protected]/T/#m2e6a57d8aeefd3146c7632e89dc36e2a0a8f68ba -- Thanks, Maxim
