From: Joe Perches <joe@perches.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Rasmus Villemoes <rv@rasmusvillemoes.dk>,
git@vger.kernel.org, jeffrey.t.kirsher@intel.com
Subject: Re: [PATCH V3] git-send-email: Add auto-cc to all body signatures
Date: Wed, 02 Dec 2015 10:20:50 -0800 [thread overview]
Message-ID: <1449080450.3716.44.camel@perches.com> (raw)
In-Reply-To: <xmqq8u5c68by.fsf@gitster.mtv.corp.google.com>
On Wed, 2015-12-02 at 09:58 -0800, Junio C Hamano wrote:
> Joe Perches <joe@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.
next prev parent reply other threads:[~2015-12-02 18:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-29 1:34 [PATCH] git-send-email: Add auto-cc to all body signatures Joe Perches
2011-07-29 1:43 ` Jeff Kirsher
2011-12-08 2:58 ` Joe Perches
2011-12-08 19:37 ` Junio C Hamano
2011-12-08 20:51 ` Joe Perches
2015-12-02 10:04 ` Rasmus Villemoes
2015-12-02 17:00 ` [PATCH V3] " Joe Perches
2015-12-02 17:58 ` Junio C Hamano
2015-12-02 18:20 ` Joe Perches [this message]
2015-12-02 18:28 ` Joe Perches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1449080450.3716.44.camel@perches.com \
--to=joe@perches.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jeffrey.t.kirsher@intel.com \
--cc=rv@rasmusvillemoes.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).