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.

Sign up for GitHub

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

Jump to bottom

Add config file parsing #60

Merged
merged 8 commits into from
Dec 5, 2019
Merged

Add config file parsing #60

merged 8 commits into from
Dec 5, 2019

Conversation

GregTheGreek
Copy link
Member

@GregTheGreek GregTheGreek commented Dec 3, 2019 •
edited

Features:

  • Adds config file parsing to the CLI
  • Adds default values for the config struct
  • Adds config file to root config.toml

Misc:

  • Suppress dead_code warnings on node/util

Future Considerations:

  • Seperate logic & structs/impl from CLI (needs research into best practices)

Closes #50, Closes #63

@ec2
Copy link
Member

ec2 commented Dec 3, 2019

Can you make this so you set the multiaddr field in the NetworkConfig?

@GregTheGreek
Copy link
Member Author

GregTheGreek commented Dec 3, 2019 •
edited

Can you make this so you set the multiaddr field in the NetworkConfig?

I'll be super honest, I added port and netowrk arbitrarily as an example. I think its better off once we get your pr in ?

Hot damn! Networking is in!

New issue for hooking CLI up to networking, and merge this based on basic structure?
otherwise happy to include that in this PR

austinabell
austinabell suggested changes Dec 3, 2019
View reviewed changes
node/src/cli/mod.rs Outdated Show resolved Hide resolved
config.toml Outdated Show resolved Hide resolved
node/src/cli/mod.rs Outdated Show resolved Hide resolved
node/src/utils/mod.rs Show resolved Hide resolved
node/src/cli/mod.rs Outdated Show resolved Hide resolved
node/src/cli/mod.rs Outdated Show resolved Hide resolved
@GregTheGreek
Copy link
Member Author

I'm going to also take down #63 per Eric's comment above

austinabell
austinabell reviewed Dec 4, 2019
View reviewed changes
node/src/cli/config.rs Outdated Show resolved Hide resolved
ec2
ec2 approved these changes Dec 4, 2019
View reviewed changes
Copy link
Member

@ec2 ec2 left a comment

Choose a reason for hiding this comment

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

me likey

@GregTheGreek GregTheGreek merged commit bbea613 into master Dec 5, 2019
@GregTheGreek GregTheGreek deleted the greg/cli/config branch December 5, 2019 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ec2 ec2 ec2 approved these changes

@ansermino ansermino ansermino approved these changes

@austinabell austinabell austinabell approved these changes

@dutterbutter dutterbutter Awaiting requested review from dutterbutter

Assignees
No one assigned
Labels
CLI Priority: 3 - Medium Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Setup Networking Config Create TOML Config
5 participants
@GregTheGreek @ec2 @ansermino @austinabell @dutterbutter