Message ID | pull.1261.v7.git.git.1657234914.gitgitgadget@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | config: introduce discovery.bare and protected config | expand |
"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.
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?
Glen Choo <chooglen@google.com> writes: > 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.