Skip to content

CLI improvements#419

Merged
cffls merged 3 commits into
0xPolygon:v0.3.0-devfrom
cffls:v0.3.0-dev-flags
Jun 7, 2022
Merged

CLI improvements#419
cffls merged 3 commits into
0xPolygon:v0.3.0-devfrom
cffls:v0.3.0-dev-flags

Conversation

@cffls

@cffls cffls commented Jun 1, 2022

Copy link
Copy Markdown
Contributor

This PR includes improvements and bug fixes as described below:

  1. Read static-nodes and trusted-nodes from default file location if not specified in CLI.
  2. Add ability to parse legacy genesis file. v0.3.0 adds multiple new fields to genesis.json, making it unable to parse old genesis file. Enable the CLI to read both new and old schema.
  3. When CLI sets a flag to an empty value in Go, e.g. 0 as to Uint64, the flag will be skipped and never set correctly. This problem is solved by using option "WithOverwriteWithEmptyValue" when merge two configs.
  4. The default non-empty value in server config will be overwritten to an empty value after flag initialization. This problem is solved by explicitly providing default value to all flags that have a default value option.

cffls added 3 commits May 31, 2022 20:30
…considered as empty

This change will fix two issues:
1. When CLI sets a flag to an empty value in Go, e.g. 0 as to Uint64, the flag will be skipped and never set correctly. This problem could be solved by using option "WithOverwriteWithEmptyValue" when merge two configs.
2. The default non-empty value in server config will be overwritten to an empty value after flag initialization. This problem is solved by explicitly providing default value to all flags that have a default value option.

@manav2401 manav2401 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me! Great work in finding the workaround for the flags issue 🚀

@cffls cffls merged commit 8808c23 into 0xPolygon:v0.3.0-dev Jun 7, 2022
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.

3 participants