Web lists-archives.com

Re: [PATCH/RFC] sequencer.c: record revert/cherry-pick commit with trailer lines




On Sun, Nov 04 2018, Nguyễn Thái Ngọc Duy wrote:

> 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/streaming

Then, 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.