Thread (3 messages) 3 messages, 2 authors, 2022-07-12

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

From: Glen Choo <hidden>
Date: 2022-07-12 22:11:39

Glen Choo [off-list ref] writes:
Junio C Hamano [off-list ref] writes:
quoted
"Glen Choo via GitGitGadget" [off-list ref] writes:
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help