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

Feature 1/merge cli config #27

Merged

Conversation

samuelvanderwaal
Copy link
Collaborator

Issue #1

Here's a first run at this. I create a cli.yml file and used clap to read config files as using structopt directly ends up with default values regardless of whether the user enters a CLI arg or not making it harder to do the manual merge. This works but I need some proper tests for it (I can do this as part of the integration tests for Issue #11) . If you don't like this approach feel free to reject PR and I can try something else.

@ThomasMeier
Copy link
Owner

@samuelvanderwaal I'll test this out today but I think this will work fine. If structopt is unneeded do you need to remove the structopt crate and comments?

@samuelvanderwaal
Copy link
Collaborator Author

structopt is still used in this implementation. clap is used to check the occurrences of each possible cli argument and overwrite values in the structopt instance before that gets passed to factom_service::run. This keeps most of your code and just adds one extra function for checking what cli args were actually submitted by the user.

@ThomasMeier ThomasMeier merged commit 12cc5b4 into ThomasMeier:master Aug 21, 2019
@ThomasMeier ThomasMeier deleted the feature-1/merge-cli-config branch August 21, 2019 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants