Re: [PATCH v7 0/5] config: introduce discovery.bare and protected config

From: Glen Choo <chooglen@google.com>
Date: Jul 12, 2022, 22:11 UTC
Message-ID: <kl6lsfn656cw.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <kl6l1quv737u.fsf@chooglen-macbookpro.roam.corp.google.com>

Glen Choo <chooglen@google.com> writes:

Show 38 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>>> This version incorporates most of Taylor's comments and suggestions. Thanks
>>> especially for the wording suggestions, I struggled with those a lot :)
>>>
>>> (I believe) I've responded upthread with my intention for each comment. The
>>> only differences between that and the actual changes are:
>>>
>>>  * In Documentation/git-config.txt, I dropped a suggestion to mention that
>>>    "git config --local" is identical to the default behavior when writing
>>>    options because I found it too hard to fit in.
>>>
>>>  * In Documentation/config/discovery.txt, I took Taylor's suggestion, but
>>>    didn't mention "discovery" for the same reasons.
>>>
>>>  * I decided to leave out the protected config lookup functions. I made some
>>>    POC patches at:
>>
>> These patches overall looked ok.  I am not very happy to see the
>> proliferation of namespaces like safe.* and discovery.* that would
>> not likely to get the second variable, though.
>
> Fair. I think `discovery.bare` is similar enough to `safe.directory`
> that it could belong in the safe.* namespace if we find a good name for
> it.
>
> We rejected "safe.bareRepository" earlier because of the insinuation
> that bare repos are unsafe. Maybe:
>
> - safe.bareDiscovery
> - safe.bareRepositoryDiscovery
> - safe.unspecifiedBareRepository
> - safe.discoveredBareRepository
>
> "safe.unspecifiedBareRepository" is sounding pretty good to me
> actually.. Any thoughts?

(+CC Johannes Schindelin for thoughts on what should go into `safe.*` and/or design considerations that went into it.)

Another thought is that `discovery.bare` and `safe.directory` should both indeed live in the same namespace, but that namespace should be named something other than `safe.*`, e.g. if we had `allowedRepositories.otherOwner` instead of `safe.directory`, it would have been a no-brainer for me to put this in the `allowedRepositories.*` namespace.

So an alternative proposal would be:

- rename this to `allowedRepositories.discoveredBare`
- (possibly not in this series, but at some point) create a
  `safe.directory` alias in that namespace, e.g.
  `allowedRepositories.otherOwner`

*But* I don't see the former making sense without the latter (I really think both should be in the same namespace), so if we think that's unnecessary churn, I'll drop this idea entirely.