public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jens Stimpfle <debian@jstimpfle•de>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] git-send-email.perl: Fix handling of suppresscc option.
Date: Wed, 12 Nov 2014 10:25:14 -0800	[thread overview]
Message-ID: <xmqqoasc46ph.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1415801891-28471-1-git-send-email-debian@jstimpfle.de> (Jens Stimpfle's message of "Wed, 12 Nov 2014 14:18:11 +0000")

Jens Stimpfle <debian@jstimpfle•de> writes:

> Signed-off-by: Jens Stimpfle <debian@jstimpfle•de>
> ---

Thanks.

Please do better than saying "Fix" to explain your changes in your
log message.

Also, on the Subject:, s/Fix/fix/; s/option./option/ to match other
entries in "git shortlog" message.

"What you think is broken" is clear (i.e. "supresscc option" is
broken) with the subject line alone, but "How it is broken", "How it
should behave instead", and "What are the differences between the
broken and the correct behaviour" should be explained in the log
message.

In other words, most of what you wrote below should come before your
S-o-b: line.

> Notes:
> ...

> diff --git a/git-send-email.perl b/git-send-email.perl
> index 9949db0..452a783 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1377,11 +1377,8 @@ foreach my $t (@files) {
>  				foreach my $addr (parse_address_line($1)) {
>  					my $qaddr = unquote_rfc2047($addr);
>  					my $saddr = sanitize_address($qaddr);
> -					if ($saddr eq $sender) {
> -						next if ($suppress_cc{'self'});
> -					} else {
> -						next if ($suppress_cc{'cc'});
> -					}
> +					next if $suppress_cc{'cc'};
> +					next if $suppress_cc{'self'} and $saddr eq $sender;

This smells more like a change in behaviour than bugfix from a
cursory look, though.  It used to be that I could receive a copy by
adding me to cc as long as I did not suppress 'self', even I
squelched everybody else by suppressing 'cc'.  I do not use such a
configuration myself but I wonder if people rely on this behaviour
as a feature.

> @@ -1425,12 +1422,9 @@ foreach my $t (@files) {
>  			my ($what, $c) = ($1, $2);
>  			chomp $c;
>  			my $sc = sanitize_address($c);
> -			if ($sc eq $sender) {
> -				next if ($suppress_cc{'self'});
> -			} else {
> -				next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
> -				next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
> -			}
> +			next if $suppress_cc{'sob'} and $what =~ /Signed-off-by/i;
> +			next if $suppress_cc{'bodycc'} and $what =~ /Cc/i;
> +			next if $suppress_cc{'self'} and $sc eq $sender;

Likewise.

I do like the updated logic flow in both hunks, though.

"When I say addresses on Cc: does not matter, it doesn't.  No matter
what the address in question is" (likewise for S-o-b:) is what the
updated logic says.  It is easier to explain than the traditional
"The way to squelch my address is by 'suppress self'; for all other
addresses on Cc:/S-o-b:, there are separate suppression methods".

But I have a slight suspicion that this special casing of 'self' was
done on purpose, and people may be relying on it.

next prev parent reply	other threads:[~2014-11-12 18:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-12 14:18 [PATCH] git-send-email.perl: Fix handling of suppresscc option Jens Stimpfle
2014-11-12 18:25 ` Junio C Hamano [this message]
2014-11-13 11:41   ` Jens Stimpfle

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqoasc46ph.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=debian@jstimpfle$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox