Re: [PATCH/RFC] sequencer.c: record revert/cherry-pick commit with trailer lines
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 6, 2018, 08:57 UTC
- Message-ID
- <87bm727fcw.fsf@evledraar.gmail.com>
- In-Reply-To
- <20181104072253.12357-1-pclouds@gmail.com>
On Sun, Nov 04 2018, Nguyễn Thái Ngọc Duy wrote:
Show 39 quoted lines
> When a commit is reverted (or cherry-picked with -x) we add an English
> sentence recording that commit id in the new commit message. Make
> these real trailer lines instead so that they are more friendly to
> parsers (especially "git interpret-trailers").
>
> A reverted commit will have a new trailer
>
> Revert: <commit-id>
>
> Similarly a cherry-picked commit with -x will have
>
> Cherry-Pick: <commit-id>
> [...]
> I think standardizing how we record commit ids in the commit message
> is a good idea. Though to be honest this started because of my irk of
> an English string "cherry picked from..." that cannot be translated.
> It might as well be a computer language that happens to look like
> English.
> [...]
> @@ -1758,16 +1757,10 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
> base_label = msg.label;
> next = parent;
> next_label = msg.parent_label;
> - strbuf_addstr(&msgbuf, "Revert \"");
> - strbuf_addstr(&msgbuf, msg.subject);
> - strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
> - strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));
> -
> - if (commit->parents && commit->parents->next) {
> - strbuf_addstr(&msgbuf, ", reversing\nchanges made to ");
> - strbuf_addstr(&msgbuf, oid_to_hex(&parent->object.oid));
> - }
> - strbuf_addstr(&msgbuf, ".\n");
> + strbuf_addf(&msgbuf, "Revert \"%s\"\n\n", msg.subject);
> +
> + strbuf_addf(&msgbuf, "Revert: %s\n",
> + oid_to_hex(&commit->object.oid));
> } else {
> const char *p;Others have already commented on the backwards-compatibility / switchover concerns, so I won't spend much time on that. Except to say that I don't think changing this would be a big deal.
Anyone trying to parse out /This reverts commit/ or other pre-set English texts we put into the commit object is already needing to deal with users changing the message. E.g. I have a habit of doing partial reverts and changing it to "This partially reverts..." etc.
Leaving aside the question of whether the pain of switching is worth it, I think it's a worthwihle to consider if we could stop hardcoding one specific human language in commit messages, and instead leave something machine-readable behind.
We do that with reverts, and also with merge commits, which could be given a similar treatment where we change e.g. "Merge branches 'jc/convert', 'jc/bigfile' and 'jc/replacing' into jc/streaming" (to use git.git's 02071b27f1 as an example) to:
Merge-branch-1: jc/convert
Merge-branch-2: jc/bigfile
Merge-branch-3: jc/replacing
Merge-branch-into: jc/streamingThen, when rendering the commit in the UI we could parse that out, and put a "Merge branches[...]" message at the top, except this time in the user's own language.