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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> On Fri, Apr 6, 2012 at 11:24 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Jeff King <peff@xxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]