Re: [PATCH v3 2/2] config: include file if remote URL matches a glob
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Nov 29, 2021, 18:33 UTC
- Message-ID
- <20211129183348.384067-1-jonathantanmy@google.com>
- In-Reply-To
- <211123.86pmqrwtf2.gmgdl@evledraar.gmail.com>
Show 28 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.)