On Fri, Sep 4, 2015 at 5:07 PM, Jeff King <peff@xxxxxxxx> wrote:
>
> Do we want to make Signed-off-by a special token here? What about "Cc:",
> and other project-specific trailers? You wouldn't want to end up with:
>
> [some comment]
> Cc: somebody
>
> Signed-off-by: somebody else
>
> It's not a problem for git-am, which should be taking in patches that
> are already signed-off (and after all, if your project does not use
> signoffs, why are you using "am -s"?). But won't "git commit -s" run
> into the same problem?
So I'm a *bit* worried about taking anything else than Signed-off-by:
exactly because of the problem you mention:
> We could look for an arbitrary rfc2822-ish string, but I'd be worried
> slightly about false positives, like:
>
> This is a paragraph with arbitrary text. But one
> of the lines will use a colon, and that a causes a
> problem: because of wrapping, this line kind of looks
> like a trailer.
which clearly needs an empty line before the sign-off.
And even if we limit it to just the last line, like you suggest:
> Maybe only the final line needs to look rfc2822-ish?
I'd still worry, for the same exact reason. The "rfc2822-ish" check is
_so_ weak that it can be trivially triggered by normal text.
So maybe it doesn't require a strict "Signed-off-by:" match, but I
think it needs something stricter than the found_rfc2822 format (like
maybe "looks like a name/email"). We just don't want to require that
*all* the lines are that way, because at least in the kernel we often
do end up adding small comments in that section.
The git project sign-off rules seem to be stricter, and it looks like
it's almost universally of the form "Signed-off-by:" with a few
"Helped-by:" and "Reviewed-by:". In the kernel, we really do tend to
be messier in that area, with the sign-off chunk not just having the
sign-offs and cc's etc, but also tends to have those occasional small
notes left by the people forwarding the emails.
And we also often don't have _strictly_ just an email. We tend to have
things like
Cc: <stable@xxxxxxxxxxxxxxx> # v3.13+
etc, and sometimes we don't have have any email at all, but things like
Reported-by: coverity
Tested-by: MysterX on #openelec
Generated-by: Coccinelle SmPL
so it's hard to give a really strict rule. It's fairly free-format.
That said, I would argue that when you apply a patch with sign-off,
pretty much by definition that patch _should_ have a sign-off from the
originator. So I suspect that the "there is an existing sign-off in
that last chunk" is a fairly good rule. Even if there are lots of
other lines too.
If you're using "git commit -s" to just commit your own work, you
presumably would normally want the extra sign-off. Or at least you can
do a "git commit --amend" fairly easily to fix things up. Doing the
amending later for "git am" is rather impractical, when it might have
been a series of 100+ emails..
Linus
--
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