Re: [PATCH v3 2/2] config: include file if remote URL matches a glob
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 29, 2021, 20:50 UTC
- Message-ID
- <211129.86zgpmelcx.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20211129183348.384067-1-jonathantanmy@google.com>
On Mon, Nov 29 2021, Jonathan Tan wrote:
Show 34 quoted lines
>> On Mon, Nov 15 2021, Jonathan Tan wrote:
>>
>> > +`hasremoteurl`::
>> > + The data that follows the keyword `hasremoteurl:` is taken to
>>
>> Both here..
>>
>> > + die(_("remote URLs cannot be configured in file directly or indirectly included by includeIf.hasremoteurl"));
>>
>> ..and here...
>>
>> > + if (skip_prefix_mem(cond, cond_len, "hasremoteurl:", &url,
>>
>> ...but not here (C code)..
>>
>> > + * For internal use. Include all includeif.hasremoteurl paths without
>>
>> ..but here..
>>
>> > +test_expect_success 'includeIf.hasremoteurl' '
>>
>> ..and also here etc., let's consistently camelCase config keys whenever
>> we're not using them for lookups in the C
>> code.
>>
>> I.e. "includeIf.hasRemoteUrl" (possibly "includeIf.hasRemoteURL"?). It
>> makes them a lot easier to read, and makes the end-user documentation &
>> messaging more consistent.
>
> The middle part is not case-insensitive, though - I tried changing it in
> the test and the test now fails. (Unless you mean that we should also
> change the code to make it case-insensitive - but I would think that
> it's better for the URL to be case-sensitive, and by extension, the
> "hasremoteurl:" part connected to it.)Ah, I forgot about that edge case. sorry. And sent [1] without having seen this as a reminder on v4. Makes sense.
(I seem to be getting really slow delivery from kernel.org to GMail these days, sometimes I can see things on lore.kernel.org hours or half a day before it pops up in my mail...)
1. https://lore.kernel.org/git/211129.864k7ug02c.gmgdl@evledraar.gmail.com/