Thread (17 messages) flat view 17 messages, 6 authors, 2018-11-07

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

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2018-11-06 08:57:40

On Sun, Nov 04 2018, Nguyễn Thái Ngọc Duy wrote:
quoted hunk ↗ jump to hunk
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help