Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Config files #2070

Merged
merged 5 commits into from
Sep 14, 2016
Merged

Config files #2070

merged 5 commits into from
Sep 14, 2016

Conversation

tomusdrw
Copy link
Collaborator

@tomusdrw tomusdrw commented Sep 11, 2016

  1. Loading ~/.parity/config.toml by default.
  2. CLI option to specify config file --config
  3. All config options are optional.
  4. CLI parameters will always override config.
  5. Docopt is no longer responsible for defaults (it wasn't possible to tell apart default value and CLI specified parameter) - flags are defined using a macro.

What might follow:
PR with "presets" - pre-defined config files for specific use cases.

Closes: #880

@tomusdrw tomusdrw added the A0-pleasereview 🤓 Pull request needs code review. label Sep 11, 2016
}
)
}
macro_rules! usage {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried tt muncher pattern here (so default and non-default fields could be mixed) but it's quickly reaching default recursion limit.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 84.996% when pulling db59d9a on config-files into edcc408 on master.

@rphmeier
Copy link
Contributor

Awesome! I can see this opening the door to nice presets for sure.

I'm not sure if keeping accounts to unlock + password files in the config is a good idea.
Also, is this file meant to be manually edited by the user to set defaults? Or is it updated automatically somewhere (I don't see it at first glance)

@rphmeier rphmeier mentioned this pull request Sep 12, 2016
@tomusdrw
Copy link
Collaborator Author

The files that are included are just examples (and are used for tests).
User can manually crate this config file and provide one using --config CLI flag.

Defaults (our defaults) are still hardcoded - but no longer in Docopt, but rather in this usage! macro.

@rphmeier
Copy link
Contributor

Is this intended to supersede #2014?

@tomusdrw
Copy link
Collaborator Author

tomusdrw commented Sep 12, 2016

Not really, afaict. #2014 specifies what auto means in some cases (like tracing, pruning). But in config you can use auto,on,off

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 84.901% when pulling 27d30fc on config-files into edcc408 on master.

@rphmeier rphmeier added the M4-core ⛓ Core client code / Rust. label Sep 13, 2016
@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 14, 2016
@arkpar
Copy link
Collaborator

arkpar commented Sep 14, 2016

LGTM. Prefer someone else to take a look also

@tomusdrw
Copy link
Collaborator Author

Please have a look at parameters definition format - it's a bit different now, maybe someone will have a nice idea how to make it more pleasant.

@rphmeier rphmeier merged commit c8533a3 into master Sep 14, 2016
@rphmeier rphmeier deleted the config-files branch September 14, 2016 13:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants