-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix(config-include): reserve more paths for future #16278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
These paths are reserved for future extension: * `.cargo/config.toml.d` -> configuration fragment support * `.cargo/user.config.toml` -> auto-include user local config * `.cargo/user.config.toml.d` -> a combination of the above This is kinda modeled after `system.conf.d`: https://www.freedesktop.org/software/systemd/man/latest/system.conf.d.html
| return Ok(None); | ||
| } | ||
|
|
||
| let reserved_paths = ["config.toml.d", "user.config.toml", "user.config.toml.d"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatives for user configuration:
local.config.tomlconfig.local.tomlconfig.user.toml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can decide at this point, we can reserve all of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be better if we reserved the whole namespace (crates/*), at least temporarily. It would move the concern of what to reserve to a later date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned about us having to guess what we might want in the future.
Would be interested in seeing an updated stabilization report as that might provide some context to help with this (e.g. what have we said about globs/directory previously?)
| return Ok(None); | ||
| } | ||
|
|
||
| let reserved_paths = ["config.toml.d", "user.config.toml", "user.config.toml.d"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For configuration fragments:
conf.dincludes- Can't think of others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These actually doesn't need to be reserved, and they don't have .toml extension so aren't allowed already.
What does this PR try to resolve?
These paths are reserved for future extension:
.cargo/config.toml.d-> configuration fragment support.cargo/user.config.toml-> auto-include user local config.cargo/user.config.toml.d-> a combination of the aboveThis is kinda modeled after
system.conf.d:https://www.freedesktop.org/software/systemd/man/latest/system.conf.d.html
How to test and review this PR?
Maybe this need some bikeshedding?
See also #t-cargo > Built-in `.cargo/config.local.toml`for non-committed config