git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Junio C HamanoNext: Duy Nguyen
Message 23 of 25 in “sequencer.c: record revert/cherry-pick commit with trailer lines”
  1. sequencer.c: record revert/cherry-pick commit with trailer linesNguyễn Thái Ngọc Duy, Nov 4, 2018
  2. Phillip WoodNov 4, 2018
  3. Duy NguyenNov 4, 2018
  4. Phillip WoodNov 4, 2018
  5. Johannes SchindelinNov 5, 2018
  6. sequencer.c: record revert/cherry-pick commit with trailer linesNguyễn Thái Ngọc Duy, Nov 4, 2018
  7. brian m. carlsonNov 4, 2018
  8. Duy NguyenNov 5, 2018
  9. Support --append-trailer in cherry-pick and revertNguyễn Thái Ngọc Duy, Nov 6, 2018
  10. Ævar Arnfjörð BjarmasonNov 6, 2018
  11. Jeff KingNov 6, 2018
  12. Jeff KingNov 6, 2018
  13. Junio C HamanoNov 7, 2018
  14. Duy NguyenNov 7, 2018
  15. Jeff KingNov 7, 2018
  16. Junio C HamanoNov 8, 2018
  17. Jeff KingNov 8, 2018
  18. Junio C HamanoNov 8, 2018
  19. Junio C HamanoNov 7, 2018
  20. Junio C HamanoNov 5, 2018
  21. Duy NguyenNov 5, 2018
  22. Junio C HamanoNov 6, 2018
  23. Ævar Arnfjörð BjarmasonNov 6, 2018
  24. Duy NguyenNov 6, 2018
  25. Ævar Arnfjörð BjarmasonNov 6, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.