Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Dec 14, 2012, 00:50 UTC
Message-ID: <CAMP44s3QtOssBDW_XRJR_K0N-rpwQ4mFx-8e2c6pUc-UkoGb1A@mail.gmail.com>
In-Reply-To: <7vsj79wmck.fsf@alter.siamese.dyndns.org>

On Thu, Dec 13, 2012 at 5:42 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 24 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Dec 13, 2012 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> One of the review points were about this piece in the test:
>>>
>>>     > +cmd=<<EOF
>>>     > +import bzrlib
>>>     > +bzrlib.initialize()
>>>     > +import bzrlib.plugin
>>>     > +bzrlib.plugin.load_plugins()
>>>     > +import bzrlib.plugins.fastimport
>>>     > +EOF
>>>     > +if ! "$PYTHON_PATH" -c "$cmd"; then
>>>
>>>     I cannot see how this could have ever worked.
>>>
>>> And I still don't see how your "would work just fine" can be true.
>>
>> As I have explained, all this code is the equivalent of python -c '',
>> or rather, it's a no-op. It works in the sense that it doesn't break
>> anything.
>
> Aren't you ashamed of yourself after having said this?

It is a fact.

Show 6 quoted lines

>> The purpose of the code is to check for the fastimport plug-in, but
>> that plug-in is not used any more, it's vestigial code, it doesn't
>> matter if the check works or not, as long as it doesn't fail.
>
> If so, the final version that is suitable for merging would have
> that unused code stripped away, no?

To the users there's absolutely no difference.

Show 9 quoted lines

>>> But it is totally a different matter to merge a crap with known
>>> breakage that is one easy fix away from the get-go.  Allowing that
>>> means that all the times we spend on reviewing patches here go
>>> wasted, discouraging reviewers.
>>
>> There is no breakage.
>
> Unused code that burdens others to read through to make sure nothing
> is broken is already broken from maintenance point of view.

Remove the whole test then. I'm already doing way more than most of the code in contrib by providing tests.

> Why are you wasting my time and everybody's bandwidth on this, when
> you are very well capable of rerolling the series with removal and
> style fixes in far shorter time?

I will do that, when I do that.

We have no time constraints, have we? This code is not getting in 1.8.1 either way.

Anyway, if you merge this code as it is, nothing bad will happen. Nobody would get hurt, and in fact, very few, if anybody, would notice.

Cheers.

-- 
Felipe Contreras