Re: [PATCH v2 4/6] refs: add REF_SKIP_REFNAME_VERIFICATION flag
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 1, 2021, 19:00 UTC
- Message-ID
- <xmqqh7bs177v.fsf@gitster.g>
- In-Reply-To
- <CAFQ2z_M35tbF6+C2MkMRm7hO8CNdUSrGTcx+8Os348+rHu4ojg@mail.gmail.com>
Han-Wen Nienhuys <hanwen@google.com> writes:
Show 15 quoted lines
> On Tue, Nov 30, 2021 at 12:31 AM Junio C Hamano <gitster@pobox.com> wrote:
>> > + if (!(flags & REF_SKIP_REFNAME_VERIFICATION) &&
>> > + ((new_oid && !is_null_oid(new_oid)) ?
>> > + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) :
>> > + !refname_is_safe(refname))) {
>>
>> So, if somebody passes REF_SKIP_REFNAME_VERIFICATION in flags, we
>> will not do the check.
>>
>> Again, like 3/6, this new bit is flipped on by test-helper
>> somewhere? Again I do not see anybody doing so in these 6 patches,
>> but I should double check.
>
> The test helper takes the flag as an argument, in decimal. If you look
> for 2048, you should find it.Awful---when the symbolic constants change in the code, the test will silently break?
It has been this way since 80f2a609 (t/helper: add test-ref-store to test ref-store functions, 2017-03-26), so it is nothing new, but at some point, we should do a better job. Even if it is used only as a tool for testing, we shouldn't have to force developers to write in assembly ;-)
Perhaps when the dust settles after this series graduates to be a part of released version. It may be a good bite-sized microproject material for aspiring new developers.