[PATCH] Don't send copies to the From: address

[PATCH] Don't send copies to the From: address

[Christian Biesinger]

Sending copies to the from address is pointless. Not
sending copies there makes it possible to do:

  git-format-patch --mbox origin
  git-send-email 00*

and get a reasonable result.

Signed-off-by: Christian Biesinger <cbiesinger@web.de>

---

 git-send-email.perl |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

486a15e29dff39ff5885d7a1e38d6c5c3b70127b
diff --git a/git-send-email.perl b/git-send-email.perl
index 3f1b3ca..31d23d6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -304,9 +304,11 @@ foreach my $t (@files) {
 					$subject = $1;
 
 				} elsif (/^(Cc|From):\s+(.*)$/) {
-					printf("(mbox) Adding cc: %s from line '%s'\n",
-						$2, $_) unless $quiet;
-					push @cc, $2;
+					unless ($2 eq $from) {
+						printf("(mbox) Adding cc: %s from line '%s'\n",
+							$2, $_) unless $quiet;
+						push @cc, $2;
+					}
 				}
 
 			} else {
@@ -335,9 +337,11 @@ foreach my $t (@files) {
 			if (/^Signed-off-by: (.*)$/i) {
 				my $c = $1;
 				chomp $c;
-				push @cc, $c;
-				printf("(sob) Adding cc: %s from line '%s'\n",
-					$c, $_) unless $quiet;
+				unless ($c eq $from) {
+					push @cc, $c;
+					printf("(sob) Adding cc: %s from line '%s'\n",
+						$c, $_) unless $quiet;
+				}
 			}
 		}
 	}
-- 
1.1.6.g71f7-dirty

Re: [PATCH] Don't send copies to the From: address

[Junio C Hamano]

Christian Biesinger <cbiesinger@web.de> writes:

> Sending copies to the from address is pointless.

Ryan, care to defend this part of the code?  This behaviour
might have been inherited from Greg's original version.

I cannot speak for Ryan or Greg, but I think the script
deliberately does this to support this workflow:

 (1) The original author sends in a patch to a subsystem
     maintainer;

 (2) The subsystem maintainer applies the patch to her tree,
     perhaps with her own sign-off and sign-offs by other people
     collected from the list.  She examines it and says this
     patch is good;

 (3) The commit is formatted and sent to higher level of the
     foodchain.  The message is CC'ed to interested parties in
     order to notify that the patch progressed in the
     foodchain.

Me, personally I do not like CC: to people on the signed-off-by
list, but dropping a note to From: person makes perfect sense to
me, if it is to notify the progress of the patch.

What you are after _might_ be not CC'ing it if it was your own
patch.  Maybe something like this would help, but even if that
is the case I suspect many people want to CC herself so it needs
to be an optional feature.

-- >8 --
[PATCH] Do not CC me

---
git diff
diff --git a/git-send-email.perl b/git-send-email.perl
index 3f1b3ca..a02e2f8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -343,7 +343,7 @@ foreach my $t (@files) {
 	}
 	close F;
 
-	$cc = join(", ", unique_email_list(@cc));
+	$cc = join(", ", unique_email_list(grep { $_ ne $from } @cc));
 
 	send_message();
 

Re: [PATCH] Don't send copies to the From: address

[Greg KH]

On Fri, Feb 10, 2006 at 07:55:13PM -0800, Junio C Hamano wrote:
> Christian Biesinger <cbiesinger@web.de> writes:
> 
> > Sending copies to the from address is pointless.
> 
> Ryan, care to defend this part of the code?  This behaviour
> might have been inherited from Greg's original version.
> 
> I cannot speak for Ryan or Greg, but I think the script
> deliberately does this to support this workflow:
> 
>  (1) The original author sends in a patch to a subsystem
>      maintainer;
> 
>  (2) The subsystem maintainer applies the patch to her tree,
>      perhaps with her own sign-off and sign-offs by other people
>      collected from the list.  She examines it and says this
>      patch is good;
> 
>  (3) The commit is formatted and sent to higher level of the
>      foodchain.  The message is CC'ed to interested parties in
>      order to notify that the patch progressed in the
>      foodchain.
> 
> Me, personally I do not like CC: to people on the signed-off-by
> list, but dropping a note to From: person makes perfect sense to
> me, if it is to notify the progress of the patch.

Yes, they specifically should be notified of the progress of their
patch.  And I like the fact that everyone else on the signed-off-by
chain also get's cc: too.  It keeps everyone in the loop so they know
what is going on.

> What you are after _might_ be not CC'ing it if it was your own
> patch.  Maybe something like this would help, but even if that
> is the case I suspect many people want to CC herself so it needs
> to be an optional feature.

Heh, getting a patch sent back to yourself this way is not a real big
deal at all :)

So, I really do not like this proposed patch at all.

thanks,

greg k-h

Re: [PATCH] Don't send copies to the From: address

[Christian Biesinger]

Junio C Hamano wrote:
> I cannot speak for Ryan or Greg, but I think the script
> deliberately does this to support this workflow:

Yeah, I suspected that this was the usecase. I don't think the patch 
breaks that, it just compares those addresses to $from, i.e. the address 
from which the email is sent.

> Me, personally I do not like CC: to people on the signed-off-by
> list, but dropping a note to From: person makes perfect sense to
> me, if it is to notify the progress of the patch.

I guess my description was a bit ambiguous, I didn't mean From: as in 
"author of the patch", but instead From: as in "the email header for the 
sender of the message", that is, the person who invokes git-send-email.

> What you are after _might_ be not CC'ing it if it was your own
> patch.  Maybe something like this would help, but even if that
> is the case I suspect many people want to CC herself so it needs
> to be an optional feature.

So a new --no-cc-self option?

> -	$cc = join(", ", unique_email_list(@cc));
> +	$cc = join(", ", unique_email_list(grep { $_ ne $from } @cc));

This seems to be basically the same as what my patch does, except that 
your way seems better :-)

Re: [PATCH] Don't send copies to the From: address

[Christian Biesinger]

Greg KH wrote:
>> Me, personally I do not like CC: to people on the signed-off-by
>> list, but dropping a note to From: person makes perfect sense to
>> me, if it is to notify the progress of the patch.
> 
> Yes, they specifically should be notified of the progress of their
> patch.  And I like the fact that everyone else on the signed-off-by
> chain also get's cc: too.  It keeps everyone in the loop so they know
> what is going on.

I didn't break that! At least, I don't think I did, and I didn't intend to.

> Heh, getting a patch sent back to yourself this way is not a real big
> deal at all :)

Maybe... but if I send the patch to a mailing list (like this one), this 
means I get it twice. I guess that's already true for replies to the 
patch, so maybe I should just live with it...

Re: [PATCH] Don't send copies to the From: address

[Ryan Anderson]

On Fri, Feb 10, 2006 at 07:55:13PM -0800, Junio C Hamano wrote:
> Christian Biesinger <cbiesinger@web.de> writes:
> 
> > Sending copies to the from address is pointless.
> 
> Ryan, care to defend this part of the code?  This behaviour
> might have been inherited from Greg's original version.
> 
> I cannot speak for Ryan or Greg, but I think the script
> deliberately does this to support this workflow:
> 
>  (1) The original author sends in a patch to a subsystem
>      maintainer;
> 
>  (2) The subsystem maintainer applies the patch to her tree,
>      perhaps with her own sign-off and sign-offs by other people
>      collected from the list.  She examines it and says this
>      patch is good;
> 
>  (3) The commit is formatted and sent to higher level of the
>      foodchain.  The message is CC'ed to interested parties in
>      order to notify that the patch progressed in the
>      foodchain.
> 
> Me, personally I do not like CC: to people on the signed-off-by
> list, but dropping a note to From: person makes perfect sense to
> me, if it is to notify the progress of the patch.

That's the thinking I've been using everytime I think about how that
code works.

> What you are after _might_ be not CC'ing it if it was your own
> patch.  Maybe something like this would help, but even if that
> is the case I suspect many people want to CC herself so it needs
> to be an optional feature.

This is probably along the right lines, but there are a few other things
we need as well.

I'm thinking of "don't add my email to cc:", as well ass "don't add cc:s
from From and Signed-off-by" as an option.

So, please feel free to commit this one, and I'll send a patch in a
minute or two for the other half.

> 
> -- >8 --
> [PATCH] Do not CC me
> 
> ---
> git diff
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 3f1b3ca..a02e2f8 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -343,7 +343,7 @@ foreach my $t (@files) {
>  	}
>  	close F;
>  
> -	$cc = join(", ", unique_email_list(@cc));
> +	$cc = join(", ", unique_email_list(grep { $_ ne $from } @cc));
>  
>  	send_message();
>  
> 
> 
> 
> 

-- 

Ryan Anderson
  sometimes Pug Majere