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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/30/2016 01:49 PM, Junio C Hamano wrote:
Junio C Hamano <gitster@xxxxxxxxx> writes:

Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:

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 <original@xxxxxxxxx>
    [fixed typo in the variable names]
    Signed-off-by: Somebhody Else <somebody@xxxxxxx>

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.

Here is an experiment I ran during my lunch break.  The script
(attached) is meant to run in the kernel repository and
for each log messages of each non-merge commit:

 * find its last paragraph, where the definition of paragraph is
   simply "a blank/empty line";

 * inspect if there is at least one RFC2822-header-looking line, or
   a line that begins with "(cherry picked from";

 * dump the ones that do not pass the above criteria.

My cursory look of the output did not spot a legitimate trailer
block that we should have identified.  The output lines shown were
ones that are not signed off at all (e.g. af8c34ce6ae32add that says
"Linux 4.7-rc2"), ones that has three-dash line "---" in them
(e.g. 133d558216d9), ones that has diffstat that should have been
after "---" (e.g. 259307074bfcf1f).

The story is the same if you run it in git.git; the "do we have at
least one rfc2822-header-looking line or '(cherry picked from' line
in the last paragraph? if so, then that is an existing trailer
block" seems to be a good heuristics to cover many cases like
these:

    d0196c8d5d3057c5c21a82f3d0113ca8e501033b
    Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
    [tomi.valkeinen@xxxxxx: resolved conflicts]
    Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>

    59f0aa9480cfef9173a648cec4537addc5f3ad94
    Link 1: https://bugzilla.kernel.org/show_bug.cgi?id=9916
            http://bugzilla.kernel.org/show_bug.cgi?id=10100
            https://lkml.org/lkml/2008/2/25/282
    Link 2: https://bugzilla.kernel.org/show_bug.cgi?id=9399
            https://bugzilla.kernel.org/show_bug.cgi?id=12461
            https://bugzilla.kernel.org/show_bug.cgi?id=11880
    Link 3: https://bugzilla.kernel.org/show_bug.cgi?id=11884
            https://bugzilla.kernel.org/show_bug.cgi?id=14081
            https://bugzilla.kernel.org/show_bug.cgi?id=14086
            https://bugzilla.kernel.org/show_bug.cgi?id=14446
    Link 4: https://bugzilla.kernel.org/show_bug.cgi?id=112911
    Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx>
    Tested-by: Chris Bainbridge <chris.bainbridge@xxxxxxxxx>
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

That sounds reasonable to me. Would a patch set to implement this new trailer block heuristic (in both sequencer and trailer) be reasonable? And if yes, should trailer know about the "(cherry picked from" prefix? (I can see it both ways - knowing about the "(cherry picked from" prefix would make it consistent with sequencer, but it seems like a detail that it shouldn't know about. Writing "Cherry-picked-from:" instead probably wouldn't solve our problem because, for backwards compatibility, we would still need to support reading the old format.)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]