Jens Stimpfle <debian@xxxxxxxxxxxx> writes:
> Signed-off-by: Jens Stimpfle <debian@xxxxxxxxxxxx>
> ---
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.
--
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