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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 27, 2012, 06:07 UTC
Message-ID: <7v394d3ffc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120727053800.GC4685@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 23 quoted lines

>> 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;
>>
>>  - 3/4 was a straight move and I didn't see anything questionable in
>>    it, but I think it would be nicer if intermediate steps can be
>>    made to still work by making 4/4 come first or something
>>    similarly simple.
>>
>> If the issues in 2/4 and 3/4 are easily fixable by going the route I
>> handwaved above, the result of doing so based on this round is ready
>> to be applied, I think.
>>
>> Eric, Jonathan, what do you think?
>
> I think this is pretty good already, though I also like your
> suggestion re 2/4.
>
> I haven't reviewed the tests these introduce and assume Eric has that
> covered.

I didn't mean to say "Unless you prove that the two suggestions are not easy to implement, I will veto the series until they are fixed." Especially, I consider that the ordering between 3 and 4 falls into the "it would be nicer if this wart weren't there" category.

The result will be queued tentatively near the tip of 'pu', but as this is primarily about git-svn, I would prefer a copy that is vetted by Eric to be fed from him.

Thanks.

P.S.

t91XX series seem to fail in 'pu' with "Can't locate Git/SVN.pm in @INC" for me. I see perl/blib/lib/Git/SVN/ directory and files under it, but there is no perl/blib/lib/Git/SVN.pm installed. I see Git/I18N.pm and Git/SVN/Ra.pm (and friends) mentioned in perl/perl.mak generated by MakeMaker, but Git/SVN.pm does not appear anywhere.

I think it is some interaction with other topics, as the tip of ms/git-svn-pm topic that parks this series does not exhibit the symptom, but it is getting late for me already, so I won't dig into this further.