Re: [RFC/PATCH 0/2] place cherry pick line below commit title

2 messages, 2 authors, 2016-09-30 · open the first message on its own page

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

From: Junio C Hamano <hidden>
Date: 2016-09-30 19:34:30

Jonathan Tan [off-list ref] writes:
quoted
I vaguely recall that there were some discussion on the definition
of "what's a trailer line" with folks from the kernel land, perhaps
while discussing the interpret-trailers topic.  IIRC, when somebody
passes an improved version along, the resulting message's trailer
block may look like this:

    Signed-off-by: Original Author [off-list ref]
    [fixed typo in the variable names]
    Signed-off-by: Somebhody Else [off-list ref]

and an obvious "wish" of theirs was to treat not just RFC2822-like
"a line that begins with token followed by a colon" but also these
short comments as part of the trailer block.  Your original wish in
[*1*] is to also treat "a line that begin with a whitespace that
follows a line that begins with token followed by a colon" as part
of the trailer block and I personally think that is a reasonable
thing to wish for, too.
If we allowed arbitrary lines in the trailer block, this would solve
my original problem, yes.
OK.
Looking at that, it seems that sequencer.c started interpreting the
last paragraph of the commit message as a footer and adding an
exception for "cherry picked from" in commit b971e04 ("sequencer.c:
always separate "(cherry picked from" from commit body",
2013-02-12). So the interpretations of sequencer.c and
interpret-trailers were already divergent, but I should have probably
at least discussed that.
It is not too late to discuss it.  I still think it is a good longer
term plan to try to unify the definition of what a trailer block is
and the implementation of the code that determines the boundary
between the log message proper and the trailer block and that allows
us to manipulate the trailer block, that currently is scattered
across multiple places into one.  Historically, "commit -s" had one
(because it needed to decide if it needs to see if the last sign-off
is already the one it is adding, and to decide if a blank line is
needed before the sing-off being added), "am -s" had another, and
"cherry-pick" probably had one, too.  "interpret-trailers" was, at
least originally, envisioned as an effort to develop a unified
machinery that can be called from these codepaths, and to aid the
development and encourage its use, it also had its own end-user
facing command.  Your interest in the "trailer" topic may be a good
trigger for us to further that original vision.
As for a reason:

1) I do not have a specific reason for placing it in that exact
position, but I would like to be able to place the "cherry picked
from" line without affecting the last paragraph (specifically, without
making the "cherry picked from" line the only line in the last
paragraph).
...
1a) (Avoiding the footer might also be a good way of more clearly
defining what the footer is. For example, currently, "cherry picked
from" is treated as a special case in sequencer.c but not in
trailer.c, as far as I can tell. If we consistently avoided the
footer, we wouldn't need such a special case anywhere.)
That is one of the numerous shortcomings of the "interpret-trailers"
that is still not finished, I would say.
2) The Linux kernel's repository has some "commit ... upstream." lines
in this position (below the commit title) - for example, in commit
dacc0987fd2e.
"A group of people seem to prefer it there" does not lead to
"therefore let's move it there for everybody".  It does open a
possibility that we may want to add a new option to put it there,
but does not justify changing what existing "-x" option does.

Re: [RFC/PATCH 0/2] place cherry pick line below commit title

From: Jonathan Tan <hidden>
Date: 2016-09-30 20:23:37

On 09/30/2016 12:34 PM, Junio C Hamano wrote:
quoted
2) The Linux kernel's repository has some "commit ... upstream." lines
in this position (below the commit title) - for example, in commit
dacc0987fd2e.
"A group of people seem to prefer it there" does not lead to
"therefore let's move it there for everybody".  It does open a
possibility that we may want to add a new option to put it there,
but does not justify changing what existing "-x" option does.
To clarify, my patch adds the new option you described (to place it 
below the title instead of at the bottom of the commit message). The 
default is still the current behavior.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help