Re: [PATCH 4/4] Move initialization of Git::SVN variables into Git::SVN.

[Michael G Schwern <schwern@pobox.com>]

On 2012.7.26 10:18 PM, Junio C Hamano wrote:
> If you swap the order of steps 3/4 and 4/4 by creating Git/SVN.pm
> that only has these variable definitions (i.e. "our $X" and "use
> vars $X") and make git-svn.perl use them from Git::SVN in the first
> step, and then do the bulk-moving (equivalent of your 3/4) in the
> second step, would it free you from having to say "it's doubtful it
> will compile by itself"?

If it wasn't clear, all tests pass with every patch using SVN 1.6.

"Compile on its own" wasn't entirely clear.  I meant that Git::SVN doesn't
depend on git-svn to set its defaults.  Git::SVN still depends on it for A LOT
of other things, and will likely remain that way for a long time, so it's
kinda splitting hairs to worry about it.

4/4 was done last to ensure the phase of git-svn when the Git::SVN globals are
initialized remains basically the same.  If they were moved into Git::SVN
before it was split out they'd be getting initialized *after* the git-svn
command has been executed.  I didn't want to expend the energy or risk the
bugs to get around that.


> In short:
> 
>  - I didn't see anything questionable in 1/4;
> 
>  - Calling up ::opt_prefix() from module in 2/4 looked ugly to me
>    but I suspect it should be easy to fix;

Originally I tried to refactor new().  It rapidly turned into a lot of work on
undocumented code with no unit tests for no use to the SVN 1.7 issue for one
variable.  This is a very cheap way to let far more important work move
forward and it has a very narrow effect.  It could be made a Git::SVN global
that git-svn grabs at, but that's not really any better.  I'd rather leave it be.


-- 
91. I am not authorized to initiate Jihad.
    -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
           http://skippyslist.com/list/