On Sat, Sep 05, 2015 at 09:30:27AM +0200, Johannes Sixt wrote:
> >How about a bit looser rule like this?
> >
> > A block of text at the end of the message, each and every
> > line in which must match "^[^: ]+:[ ]" (that is,
> > a "keyword" that does not contain a whitespace nor a colon,
> > followed by a colon and whitespace, and arbitrary value thru
> > the end of line) is a signature block.
>
> Why do we need a new rule? The old git-am had a logic that pleased everyone,
> and it must have been implemented somewhere. Shouldn't it be sufficient to
> just re-implement or re-use that logic?
That was my thought, too; if there is a regression, let's start by
fixing that for the upcoming 2.6.0, and then we can worry about doing
something fancier[1] later.
And I think the original behavior really is what Linus is asking for: we
consider the final block, even with a "[]" comment, as a S-o-b block if
it has a Signed-off-by in it. So if we have the final block:
[some comment]
Signed-off-by: Somebody Else <them@xxxxxxxxxxx>
Running "am -s" with the current master yields:
[some comment]
Signed-off-by: Somebody Else <them@xxxxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
which is wrong. Running the same with v2.5.0 gets:
[some comment]
Signed-off-by: Somebody Else <them@xxxxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
So far so good. Now let's change our input to:
[some comment]
Reviewed-by: Somebody Else <them@xxxxxxxxxxx>
and run "am -s". Current "master" continues to stick the extra line in
there. No surprise. But now so does v2.5.0!
So I don't think the old behavior covered all cases well, either, and
there's room for improvement. But likewise, I don't recall seeing a lot
of complaints about it in practice. It seems like a sane thing to
restore it for the upcoming release, and then build from there.
-Peff
[1] I think part of the reason people are interested in "fancy" here is
that this topic extends beyond "git am". There's "commit -s", of
course, but there's also the generic "interpret-trailers" command,
which is supposed to be a generalization of the "--signoff" option.
It would be nice if the rules remained consistent for when we add a
trailer to an existing block, rather than special-casing signoffs.
But again, I think that's something to shoot for in the long run.
It's more important in the short term not to regress "am".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html