Re: [PATCH 2/4] Prepare Git::SVN for extraction into its own file.

From: Michael G Schwern <schwern@pobox.com>
Date: Jul 27, 2012, 08:16 UTC
Message-ID: <50124E71.2010302@pobox.com>
In-Reply-To: <7vvch93hpy.fsf@alter.siamese.dyndns.org>

On 2012.7.26 10:18 PM, Junio C Hamano wrote:

> Forgot to sign-off, or are you still unsure about this step?

I just never think to do it. It's just a line in the commit message, right? There's no crypto involved like tag -s. Is it a blocker? I guess I can write a msg-filter if it's important.

Show 7 quoted lines

> Again, I agree with you that passing $prefix as one of the arguments
> to ->new is the right thing to do in the final state after applying
> the whole series.  I don't know if later steps in your patch series
> will do so, but it _might_ make more sense to update ->new and its
> callers to do so without doing anything else first, so that you do
> not have to call out to the ::opt_prefix() when you split things
> out.

I don't personally plan on doing any more about it, no. It isn't needed for SVN 1.7, there's very little real code change (which you could see by looking at my remote instead of waiting to be fed patches...) and its a very, very minor problem in the grand scheme.

How git-svn structures its switches needs a ton of work, and there are far deeper problems with Git::SVN. For one, it's completely undocumented. For another, Git::SVN can't instantiate an object without git-svn being loaded and so is very difficult to unit test. I wouldn't want to change the constructor interface until I could construct an object.

The first step toward that would be to change git-svn so it can be loaded as a library using the standard "main() unless caller" trick. Then Git::SVN unit tests can require git-svn as a library without executing it and get some tests written with a minimum of Git::SVN code change.

Step zero would be to allow Perl unit tests to either use or emulate the work done in lib-git-svn.sh. The major problem being how to communicate the location of the trash directory, currently done by environment variables. A simple trick would be for the Perl tests to execute a shell wrapper that outputs the relevant information.

None of which I plan to get into just now.

-- 
emacs -- THAT'S NO EDITOR... IT'S AN OPERATING SYSTEM!