git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Apr 6, 2012, 22:22 UTC
Message-ID
<CAMP44s07JOVQp2xNNdPew5L5_ibyk713m7UoJ6MrKOyKEmTkGw@mail.gmail.com>
In-Reply-To
<20120406214219.GB5436@sigill.intra.peff.net>
On Sat, Apr 7, 2012 at 12:42 AM, Jeff King <peff@peff.net> wrote:
Show 32 quoted lines
> 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@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.

My comment was to Junio, not to you; Junio said "sometimes something is worse than nothing"; if that's not preferring nothing, I don't know what it is.

I just sent an email where I assumed you preferred the status quo, sorry if I misinterpreted you, but my last patches to add some tests were never applied because you advocated the status quo; no tests.

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

Did you notice the "RFC" part of it? This patch was not meant to be applied as-is.

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

I have already done the "hard part" in the past and yet my patches were dropped because of commit message wording (or whatever), and we ended up with the status quo; no tests. So excuse me if I'm not eager to engage in a discussion in which you provide some "constructive criticism", Junio agrees, I disagree, and the patch never goes anywhere.

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

I didn't say there was no problem with the patch, I said the *particular* problem that you pointed out was incorrect (the shebang).

But OK, I will try once more to provide a more proper solution, let's see how it goes.

Cheers.
-- 
Felipe Contreras
Previous: Jeff KingNext: Junio C Hamano
Message 6 of 18 in “tests: add initial bash completion tests”
  1. tests: add initial bash completion testsFelipe Contreras, Apr 6, 2012
  2. Jeff KingApr 6, 2012
  3. Junio C HamanoApr 6, 2012
  4. Felipe ContrerasApr 6, 2012
  5. Jeff KingApr 6, 2012
  6. Felipe ContrerasApr 6, 2012
  7. Junio C HamanoApr 6, 2012
  8. Felipe ContrerasApr 6, 2012
  9. Jeff KingApr 6, 2012
  10. Felipe ContrerasApr 6, 2012
  11. Junio C HamanoApr 6, 2012
  12. Felipe ContrerasApr 6, 2012
  13. Jeff KingApr 6, 2012
  14. Felipe ContrerasApr 6, 2012
  15. Junio C HamanoApr 6, 2012
  16. Junio C HamanoApr 6, 2012
  17. Felipe ContrerasApr 7, 2012
  18. Jeff KingApr 6, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.