On Tue, 6 Sep 2011 13:57:50 -0700
Eric Wong <normalperson@xxxxxxxx> wrote:
> Bryan Jacobs <bjacobs@xxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html