Re: [PATCH] git-send-email.perl: fix In-Reply-To for second and subsequent patches

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 14 Oct 2010 13:22:50 -0500
Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:

> (+cc: some send-email people)
>

For the new recipients, the original mail is here btw:
http://permalink.gmane.org/gmane.comp.version-control.git/159039

More comments below.

> Hi,
> 
> Antonio Ospite wrote:
> 
> > Make second and subsequent patches appear as replies to the first patch,
> > even when an initial In-Reply-To is supplied
> [...]
> > Signed-off-by: Antonio Ospite <ospite@xxxxxxxxxxxxxxxxx>
> 
> Thanks.
>

Thanks for commenting Jonathan.

> >   - When $initial_reply_to is asked to the user, it is asked as the
> >     "Message-ID to be used as In-Reply-To for the _first_ email", this
> >     makes me think that the second and subsequent patches are not using
> >     it
> 
> This kind of justification belongs in the commit message, no?
> That way, we can save future readers the trouble of figuring out
> the rationale all over again when considering future changes to this
> code.
>

Ok, I can add this in the commit message, I am waiting some days for
v2, in case someone else has more to say.

> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -1313,7 +1313,7 @@ foreach my $t (@files) {
> >  
> >  	# set up for the next message
> >  	if ($thread && $message_was_sent &&
> > -		(chain_reply_to() || !defined $reply_to || length($reply_to) == 0)) {
> > +		($message_num == 1 || chain_reply_to() || !defined $reply_to || length($reply_to) == 0)) {
> >  		$reply_to = $message_id;
> 
> Would it be possible to break this long line?
>

I like the OR chain on the same line, but I can split it anyways if
that's the preference.

> If you're feeling particularly adventurous, it would be nice to add a
> test for the changed functionality to t/t9001-send-email.sh, so we
> don't break it with other changes in the future.
>

No promises, but I might give that a try.

> I haven't looked too deeply or even tried running applying the patch,
> but generally it looks good to me.
> 
> Ciao,
> Jonathan
> 

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Attachment: pgpiI5CpFLj7G.pgp
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]