-
Notifications
You must be signed in to change notification settings - Fork 925
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
node: updates for node.Config #74
Conversation
… pointers causes complexities
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with these changes. I don't really like passing the core config directly to the new full node but it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼
… will be resolved in another way
I've moved back the Config to node, as there is another solution to dep cycle and I still prefer to keep Config in the node package, so it is clear that is related to the node by just looking into |
Also, I've changed the toml parser to BurntSushi/toml from pelletier/go-toml/v2 as the latter does not support Config parsing of Tendermint config needed in #81, as having two parsers is not good, even if the latter is much better in terms of performance. |
3fcdc45
to
3ee8d31
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏼 nothing to complain about.
Shipit-tipihS |
Context
Work on #30 turns out quite big and to follow the practice of keeping PR relatively small, I've extracted some work related to configs into separate PR
Changes