Skip to content
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

Support for cascading configurations #93

Closed
epage opened this issue Mar 11, 2019 · 2 comments · Fixed by #111
Closed

Support for cascading configurations #93

epage opened this issue Mar 11, 2019 · 2 comments · Fixed by #111
Assignees
Labels
enhancement Improve the expected

Comments

@epage
Copy link
Collaborator

epage commented Mar 11, 2019

The idea being that we merge the configs from the following locations

  • CLI flags
  • manifest path's directory
  • either every parent directory or the workspace root
  • home
  • env variables (where does this fall in precedent order?)
  • defaults

Use cases

  • Centralize most release knowledge with overrides per repo in a mono repo
  • Allow some settings to be left up to the user

Questions

  • Should replacement file paths be relative to the manifest path or the release config?

Possible implementation

  • Mergable struct
    • Switch all config settings to Option<_>
    • Have a merge function
    • Have member functions that unwrap_or the field's default
    • Have a function that creates the struct with all defaults applied
  • Cascading struct
    • Create a struct ConfigSource of Option<&_>
    • Have conversions from ConfigFile, ConfigArgs etc to ConfigSource
    • Create a Config struct that orders all of the potential sources
  • Cascading trait
  • Mergble trait
    • Config struct with non-Option members
    • Config::from_sources(&[&dyn ConfigSource])
  • Load it all into a hash map, perform hash map merges, and then move to the current config struct. The downside is this defers error reporting, making it so you can't report which file caused the problem
@epage epage added the question Uncertainty is involved label Mar 11, 2019
@sunng87
Copy link
Collaborator

sunng87 commented Mar 15, 2019

Currently we support the config source from:

  • cli options: top priority
  • release.toml in project root
  • package.metadata.release section in Cargo.toml
  • .release.toml under HOME: used when neither release.toml nor package.metadata.release configured

As we have workspace supported, it makes sense to support release.toml or meradata.release from workspace parent or root.

Should replacement file paths be relative to the manifest path or the release config?

I think it should be relative to where it configured.

Have a merge function

Good catch because items in replacement should be concat together instead of overwrited.

I'm +1 for the first implementation proposal. It may also simplify the big section in main.rs

@jan-auer jan-auer mentioned this issue Mar 17, 2019
6 tasks
@epage epage added enhancement Improve the expected and removed question Uncertainty is involved labels Apr 4, 2019
@epage epage self-assigned this Apr 4, 2019
@epage epage changed the title Should we support cascading configurations? Support for cascading configurations Apr 4, 2019
@epage
Copy link
Collaborator Author

epage commented May 7, 2019

/// 2. $(pwd)/Cargo.toml package.metadata.release (with deprecation warning)

I'm not seeing this deprecation warning.

epage added a commit to epage/cargo-release that referenced this issue May 8, 2019
Now we'll load from all config sources, merging the result.

Fixes crate-ci#93
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve the expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants