threads / patch / 3296

patchDon't send copies to the From: address

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

## tl;dr

6 messages between Feb 11, 2006 and Feb 13, 2006. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Christian Biesinger· Feb 11, 2006, 02:47 UTC · lore

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
Show changes to git-send-email.perl +10 −6
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
Junio C Hamano· Feb 11, 2006, 03:55 UTC · re: Christian Biesinger · lore

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

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

Show changes to git-send-email.perl +1 −1
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();
Greg KH· Feb 11, 2006, 04:52 UTC · re: Junio C Hamano · lore

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

On Fri, Feb 10, 2006 at 07:55:13PM -0800, Junio C Hamano wrote:
Show 26 quoted lines
> 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
Christian Biesinger· Feb 11, 2006, 12:33 UTC · re: Greg KH · lore

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

Greg KH wrote:
Show 8 quoted lines
>> 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...

Christian Biesinger· Feb 11, 2006, 12:31 UTC · re: Junio C Hamano · lore

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

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 :-)

Ryan Anderson· Feb 13, 2006, 07:20 UTC · re: Junio C Hamano · lore

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

On Fri, Feb 10, 2006 at 07:55:13PM -0800, Junio C Hamano wrote:
Show 26 quoted lines
> 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.

Show 23 quoted lines
> 
> -- >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

← back to recent threads