Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo
- From
- Bryan Jacobs <bjacobs@woti.com>
- Date
- Sep 7, 2011, 14:14 UTC
- Message-ID
- <20110907101434.281d037f@robyn.woti.com>
- In-Reply-To
- <20110906205750.GB12574@dcvr.yhbt.net>
On Tue, 6 Sep 2011 13:57:50 -0700 Eric Wong <normalperson@yhbt.net> wrote:
Show 7 quoted lines
> 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.
Show 9 quoted lines
> > +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.
Show 10 quoted lines
> > + 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.
Show 7 quoted lines
> > +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