git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)
Date: Thu, 13 Dec 2012 15:42:19 -0800	[thread overview]
Message-ID: <7vsj79wmck.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CAMP44s0qK6yNiPe0ERDJWK-wfm3DdXZYwRzisoCPJ7PjsdkObQ@mail.gmail.com> (Felipe Contreras's message of "Thu, 13 Dec 2012 16:05:08 -0600")

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?

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

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

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?

  reply	other threads:[~2012-12-13 23:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-12 23:58 What's cooking in git.git (Dec 2012, #03; Wed, 12) Junio C Hamano
2012-12-13  6:08 ` Felipe Contreras
2012-12-13  8:11   ` Junio C Hamano
2012-12-13 10:08     ` Felipe Contreras
2012-12-13 12:04       ` Max Horn
2012-12-13 19:06         ` Felipe Contreras
2012-12-14 13:11           ` Max Horn
2012-12-15  3:14             ` Felipe Contreras
2012-12-15  7:09               ` Michael Haggerty
2012-12-15  7:45                 ` Felipe Contreras
2012-12-15  8:44                   ` Nguyen Thai Ngoc Duy
2012-12-15  9:24                     ` Felipe Contreras
2012-12-13 19:31       ` Junio C Hamano
2012-12-13 22:05         ` Felipe Contreras
2012-12-13 23:42           ` Junio C Hamano [this message]
2012-12-14  0:50             ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vsj79wmck.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).