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

Add app config file support #7

Closed
avive opened this issue Dec 27, 2017 · 6 comments
Closed

Add app config file support #7

avive opened this issue Dec 27, 2017 · 6 comments

Comments

@avive
Copy link
Contributor

avive commented Dec 27, 2017

Implement support for app configuration via a config file, and write tests for app configs using a file.
Choose TOML or YAML as file format and integrate with cli1.v1

@avive
Copy link
Contributor Author

avive commented Jan 10, 2018

@jreyeshdez
Copy link
Contributor

I am working on this feature and came across same issue as pointed here:
urfave/cli#599

It looks like it is a bug and still open

@y0sher
Copy link
Contributor

y0sher commented Jan 18, 2018

Hi @jreyeshdez,
I'm also working on this here : #20, I tried to overcome some limitations caused by the cli parser bugs.
I'll push it to a branch and edit this comment so we can combine forces :)

we might want to merge those issues @avive ?

Edit:
you can look at the code here https://github.com/y0sher/go-spacemesh/tree/config
I took care of uint loading with a map and a simple casting func.
The problem is as soon as we let a duration value inside there's a type mismatch error because it isn't a duration. (it's either a string or an int ex: "24h5m" or 86400000 (miliseconds) )
for some reason even though I took care of parsing durations by ourselves, the rest string and boolean flags get messed up.
if we comment out durations everything works great.
some other issues :

  • We must use the names we use as flag names in the toml file. that means we can't use multiple names as 'flagname, fn' for command line parsing
  • Using sections doesn't work

I'm thinking maybe we should drop using the cli for that and load a config file by ourself then apply it to flags. to make sure all types are working and everything is consistent without relying on the cli package.

Have any thoughts or ideas ? @jreyeshdez @avive

@avive
Copy link
Contributor Author

avive commented Jan 23, 2018

@y0sher - looking into this today

@avive
Copy link
Contributor Author

avive commented Jan 23, 2018

@y0sher - do you mind opening a PR from your forked code so I can have a look and comment? Just please mark it as WIP

@avive
Copy link
Contributor Author

avive commented Jan 29, 2018

Thanks @y0sher for the good work on getting this done!

@avive avive closed this as completed Jan 29, 2018
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

No branches or pull requests

3 participants