On Wed, 2015-12-02 at 09:58 -0800, Junio C Hamano wrote:
> Joe Perches <j...@perches.com> writes:
>
> > Many types of signatures are used by various projects.
> >
> > The most common type is formatted:
> > "[some_signature_type]-by: First Last <email <at> domain.tld>"
> > e.g:
> > "Reported-by: First Last <email <at> domain.tld>" (no quotes are used)
> >
> > Make git-send-email use these signatures as "CC:" entries.
> >
> > Add command line option --suppress-cc=signatures to avoid
> > adding these entries to the cc.
> >
> > Signed-off-by: Joe Perches perches.com>
> > Acked-by: Jeff Kirsher intel.com>
>
> I wonder what send-email with this patch does to the above two lines
> with "" not "@" ;-) How was this patch sent?
gnome evolution v3.18.2 email client.
And it seems all newer versions of evolution beyond 3.12
are really, really poor at sending inline patches. <grumble>
I'll update and resend using git-send-email eventually
> In any case, did you mean "Helped-by:" not "Acked-by:"? "git
> shortlog git-send-email.perl" does not show that name as one of the
> major stakeholders who would be capable of giving an Ack on it.
At least for linux-kernel, "Acked-by:" doesn't mean a maintainer
or a contributor to a particular module/file, just someone that
has looked at the patch, tried it, and approved of the concept.
I don't know what process git uses for approval/signatures.
> > ---
> > > It's been four years, but I recently ran into this. I mistakenly thought
> > > that git would actually pick up cc addresses also from Reported-by, so
> > > the reporter ended up not being cc'ed. Is there any chance this could be
> > > revisited,
> >
> > Here's a refresh if desired. I still think it's sensible.
>
> What the patch tries to achieve may make a lot of sense. I however
> do not necessarily think this particular implementation does,
> unfortunately.
>
> These "Random-by:", especially the ones that the author adds on his
> own initiative like "Reported-by:", are often followed by just a
> name but not an addresses. A "Signed-off-by:" and "Cc:" that is not
> followed by a valid e-mail address may deserve to get an error (or
> perhaps an end-user interaction "This is not a valid address. What
> do you want to do about it?") so "/^(Signed-off-by|Cc): (.*)$/i"
> does not need its own sanity check on $2, because a later call to
> extract-valid-address or extract-valid-address-or-die will take care
> of it.
> It would however be wrong to cause the program to error out or even
> bother the user upon seeing such random trailer lines that the
> author did not mean to have an e-mail address on it in the first
> place. If you have a trailer line
>
> Random-by: Joe Perches
>
> without an address, I suspect you will end up adding "Joe" and
> "Perches" as two addresses on the Cc: line, which is most likely not
> what the user intended [*1*].
At least with new versions of git-send-email.perl
that's true so the patch will need to validate that
there is an email address following.
> As to the lingo, these are still not signatures, but during the past
> years, it seems that we settled on using the term "trailers" for
> these e-mail header-like things at the end of the log message.
> "Trailers" are not limited to "*-by:" so this patch is not about
> adding auto-cc to all trailers--a retitle would be
>
> send-email: add auto-cc to addresses that appear on *-by: trailers
>
> or something (and the option and variable names may need to be
> updated to match).
>
>
> [Footnote]
>
> *1* I further suspect that the existing code shares a similar issue.
> Don't Cc: and Signed-off-by: expect a single address on each line in
> the usual fashion? Perhaps a two-patch series whose first part does
>
> - if (/^(Signed-off-by|Cc): (.*)$/i) {
> + if (/^(Signed-off-by|Cc): (.*<[^>]*>)\s*$/i) {
>
> to tighten it (so that "Cc: Joe Perches" would not result in two
> pieces of mail sent to Joe and Perches), with your patch as a follow
> up, may be a good way forward.
>
> I dunno.
I believe the old git-send-email code required addresses
and validated the form after Signed-off-by:'s.
I haven't looked at the code for several years and just
refreshed it without much thinking or testing.
I'll do a bit more and resend.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
