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

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

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.


> 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!