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

[Bryan Jacobs <bjacobs@woti.com>]

On Tue, 6 Sep 2011 13:57:50 -0700
Eric Wong <normalperson@yhbt.net> wrote:

> Bryan Jacobs <bjacobs@woti.com> wrote:
> > +sub split_merge_info_range {
> > +	my ($range) = @_;
> > +	if ($range =~ /(\d+)-(\d+)/o) {
> 
> No need for "/o" in regexps unless you have a (constant) variable
> expansion in there.

Okay, I'll take that out. I got into the habit of putting "optimize" on
all regexes without an explicitly dynamic variable on some earlier Perl
version.

> > +sub merge_commit_fail {
> > +	my ($gs, $linear_refs, $d) = @_;
> > +	#while (1) {
> > +	#	my $cs = shift @$linear_refs or last;
> > +	#	command_noisy(qw/cherry-pick/, $cs);
> > +	#}
> > +	#command_noisy(qw/cherry-pick -m/, '1', $d);
> 
> Huh?  If there's commented-out code, it must be explained or removed.

I think I did explain that in my earlier comments. I'm still not happy
with the recovery-from-aborted-commit-series handling. That commented
bit was my attempt.

The best suggestion so far is to prescan the commits to fail-fast. I
will do that in the next revision of the patch, just give me some time
to put it together.

> > +	fatal "Aborted after failed dcommit of merge revision";
> > +}
> 
> > +++ b/t/t9160-git-svn-mergeinfo-push.sh
> > @@ -0,0 +1,97 @@
> > +#!/bin/sh
> > +#
> > +# Copyright (c) 2007, 2009 Sam Vilain
> 
> That should be: "Copyright (c) 2011 Brian Jacobs", correct?

Well, the file was copied from one bearing the Vilain copyright bit.
I'm not sure I entirely understand why it matters who holds the
individual copyrights if you have a collective license which is going to
be changed, but I can't just stick my own name on derived work - as the
setup code for that unit is.

> > +test_expect_success 'check svn:mergeinfo' '
> > +	mergeinfo=$(svn_cmd propget svn:mergeinfo
> > "$svnrepo"/branches/svnb1)
> > +	echo "$mergeinfo"
> 
> No need to echo unless you're debugging a test, right?
> 

Correct, leftover test-debugging cruft, will remove.

I will submit another revision shortly.

Bryan Jacobs