Re: [PATCH v2] git-svn: teach git-svn to populate svn:mergeinfo

From: Bryan Jacobs <bjacobs@woti.com>
Date: Sep 12, 2011, 14:24 UTC
Message-ID: <20110912102422.21aa6570@robyn.woti.com>
In-Reply-To: <20110909222159.GA6530@dcvr.yhbt.net>

On Fri, 9 Sep 2011 15:21:59 -0700 Eric Wong <normalperson@yhbt.net> wrote:

Show 14 quoted lines

> Some comments inline, but I can clean them up myself and push out in a
> bit.
> 
> > --- a/Documentation/git-svn.txt
> > +++ b/Documentation/git-svn.txt
> > @@ -213,6 +213,14 @@ discouraged.
> >  	store this information (as a property), and svn clients
> > starting from version 1.5 can make use of it. 'git svn' currently
> > does not use it and does not set it automatically.
> 
> I noticed this conflicts when applying due to the missing --mergeinfo=
> documentation.  Did you intend to remove --mergeinfo entirely and
> replace it with this?  I think some folks already depend on it (it's
> been around since last year).

No, that was me munging my patch base. Feel free to set the documentation for mergeinfo however you like, even documenting this feature if you wish. The functionality of --mergeinfo still should work with this patch, although obviously if it's provided the auto-population will be bypassed.

Show 9 quoted lines

> > +	if (not defined($push_merge_info)
> > +			or $push_merge_info eq "false"
> > +			or $push_merge_info eq "no"
> > +			or $push_merge_info eq "never") {
> 
> I missed this the first time, but "||" and "!" are easier for
> C programmers to understand and higher in precedence (ref: perlop
> manpage)
>

You're right, "||" and "!" are better here. TMTOWTDI is the Perl motto, no? A double-edged sword.

Thanks for the help and review. I'll let you do the cleanup and push the resulting patch when you feel it's ready (as you suggested), rather than iterating again for these small changes.

Bryan Jacobs