Re: [PATCH 2/2] refs: loosen restriction on wildcard "*" refspecs

From: Jacob Keller <jacob.keller@gmail.com>
Date: Jul 23, 2015, 16:51 UTC
Message-ID: <CA+P7+xq8gSMYRD2SMNqDz8WL+A8y1jBPTBMQG_ckP-CeXk8X2A@mail.gmail.com>
In-Reply-To: <xmqqa8umrfge.fsf@gitster.dls.corp.google.com>

On Thu, Jul 23, 2015 at 9:44 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> Jacob Keller <jacob.keller@gmail.com> writes:
>
>>> By the way, have you run test suite before sending this (or any
>>> previous round of this) patch?  This seems to break t5511-refspec.sh
>>> for me.
>>
>> Looks like another location I forgot to update. I can send a re-spin
>> if you need with the following diff. Basically looks like the tests
>> just didn't get updated to count the new behavior is valid.
>
> Yeah, basically looks like an untested patch was sent and nobody
> noticed during earlier rounds, even the patch was rerolled a few
> times, before I finally took it to 'pu' to take a look.  A typical
> slow summer moment---people rightfully find it is more important to
> have fun themselves than to help polishing others' patches ;-).
>

I think what happened, is that I ran some tests when the patch was "configurable" and I had modified the one set of tests to try with the option enabled, but it didn't fail in the t5511-refspec.sh since this series of tests didn't enable the new option. Then, I never re-tested again (OOPS!) when I removed the optional portion.

Show 5 quoted lines

> Will squash the changes; no need to resend (unless people discover
> other issues; let's hope that I wouldn't be the one to do so ;-).
>
> Thanks.
>

Thank you! :)

Regards, Jake

Show 30 quoted lines

>> diff --git i/t/t5511-refspec.sh w/t/t5511-refspec.sh
>> index de6db86ccff0..7bfca7962d41 100755
>> --- i/t/t5511-refspec.sh
>> +++ w/t/t5511-refspec.sh
>> @@ -71,11 +71,11 @@ test_refspec fetch ':refs/remotes/frotz/HEAD-to-me'
>>  test_refspec push ':refs/remotes/frotz/delete me'              invalid
>>  test_refspec fetch ':refs/remotes/frotz/HEAD to me'            invalid
>>
>> -test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' invalid
>> -test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*-blah' invalid
>> +test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*-blah'
>> +test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*-blah'
>>
>> -test_refspec fetch 'refs/heads*/for-linus:refs/remotes/mine/*' invalid
>> -test_refspec push 'refs/heads*/for-linus:refs/remotes/mine/*' invalid
>> +test_refspec fetch 'refs/heads*/for-linus:refs/remotes/mine/*'
>> +test_refspec push 'refs/heads*/for-linus:refs/remotes/mine/*'
>>
>>  test_refspec fetch 'refs/heads/*/*/for-linus:refs/remotes/mine/*' invalid
>>  test_refspec push 'refs/heads/*/*/for-linus:refs/remotes/mine/*' invalid
>>
>>
>>
>>
>> Regards,
>> Jake
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html