Re: [RFC/PATCH] tests: add initial bash completion tests

From: Jeff King <peff@peff.net>
Date: Apr 6, 2012, 21:42 UTC
Message-ID: <20120406214219.GB5436@sigill.intra.peff.net>
In-Reply-To: <CAMP44s0xAvc9aTeBP81tXhX6Q67+7cQ-51C1AWrKPN7bc7=JXQ@mail.gmail.com>

On Sat, Apr 07, 2012 at 12:28:05AM +0300, Felipe Contreras wrote:

Show 20 quoted lines

> On Fri, Apr 6, 2012 at 11:24 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Jeff King <peff@peff.net> writes:
> >
> >> On Fri, Apr 06, 2012 at 10:28:39PM +0300, Felipe Contreras wrote:
> >>
> >>> Something is better than nothing.
> >>
> >> Yes, but...
> >
> > ;-)
> >
> > This is a good example that sometimes something is worse than nothing,
> > unless watched carefully by a competent reviewer.
> 
> And this is a good example of why you shouldn't blindly trust what a
> 'competent reviewer' says, as I'm pretty sure the comment is wrong.
> 
> But hey, if you prefer nothing, fine, drop this patch; let's continue
> to blindly modify the completion and fix regressions as they come. I
> guess I should drop my other tests as well.

Sorry, but I think you are wrong, and this is a perfect example of why you are sometimes frustrating to work with. Your patch is definitely a move in the right direction, and we would love to have something like it as part of git. And I'm sure it runs fine under _your_ setup. But the git community is much larger than just your setup, and your patch is a regression for other people, as it breaks "make test".

Did I say "let's throw away this patch"? No. I said "here is a problem with your patch", and I even sketched out a fix.

And nor do I think Junio was saying "let's throw it out". I think he was responding specifically to "something is better than nothing". It's _not_ better if it regresses other cases. So the patch as-is is not acceptable, but it could be made so.

But instead of taking the constructive criticism and iterating on your patch, you are ready to withdraw your patch. That seems silly when you have already done the hard part, and the fix to make the patch acceptable is the easy part.

But maybe I am wrong. Maybe there is no problem at all with your patch, and my analysis is wrong, and yours is right. I am willing to admit that as a possibility. But let's discuss it in the other part of the thread and find out, shall we?

-Peff