CLI options and commands renaming #618
Conversation
…mance # Conflicts: # pantheon/src/test/resources/everything_config.toml
- fixes NC-2031 and NC-2032 by splitting RPC listen into host and port - moves default values to DefaultCommandValues interface - statically imports defaults - renames some rpc variables tu include http or ws for homogeneity and clarity - fixes Refresh delay error message as it should be "Refresh delay" and not the variable name - updates unit tests and yaml test configs
split p2p and metrics listen options into host and port
NicolasMassart
added
documentation
| /** @deprecated Deprecated in favour of --network option */ | ||
| @Deprecated | ||
| @Option( | ||
| hidden = true, |
There was a problem hiding this comment.
This probably shouldn't be hidden until we actually support setting the network ID in genesis config. Otherwise there's no way to set a custom network ID.
| String.valueOf(DEFAULT_MAX_REFRESH_DELAY))); | ||
| "Refresh delay must be a positive integer between %s and %s", | ||
| String.valueOf(DefaultCommandValues.DEFAULT_MIN_REFRESH_DELAY), | ||
| String.valueOf(DefaultCommandValues.DEFAULT_MAX_REFRESH_DELAY))); |
There was a problem hiding this comment.
nit: Static import for all the DefaultCommandValues references would improve readability.
| @Option( | ||
| hidden = true, |
There was a problem hiding this comment.
We need to keep this unhidden until DEV is an option for --network.
| @@ -573,38 +625,38 @@ private static int trueCount(final Boolean... b) { | |||
| } | |||
| } | |||
|
|
|||
| private File getNodePrivateKeyFile() { | |||
| private File getNodePrivateKeyFile(@Nullable final File nodePrivateKeyFile) { | |||
There was a problem hiding this comment.
Pantheon doesn't currently use @Nullable so better to leave it off for consistency.
| // "Synchronization mode (Value can be one of ${COMPLETION-CANDIDATES}, default: | ||
| // ${DEFAULT-VALUE})" | ||
| // ) | ||
| private final SyncMode syncMode = DEFAULT_SYNC_MODE; |
There was a problem hiding this comment.
It would be quite helpful if we kept this as-is - I have another open PR that starts using it again. :)
There was a problem hiding this comment.
It's me, deciding to remove dead code just before it gets to life gain after month being dead...
| checkNotNull(syncMode); | ||
| synchronizerConfigurationBuilder.syncMode(syncMode); | ||
| private SynchronizerConfiguration buildSyncConfig() { | ||
| synchronizerConfigurationBuilder.syncMode(DEFAULT_SYNC_MODE); |
There was a problem hiding this comment.
It would be good not to remove the sync-mode flag. I have an open PR that starts using it again. :)
| * Public key export sub-command | ||
| * | ||
| * <p>Export of the public key takes a file as parameter to export directly by writing the key in | ||
| * the file. A direct output of the key to sdt out is not done because we don't want the key value |
There was a problem hiding this comment.
sdt is an unusual abbreviation (at least to me). stdout or the full standard out is probably better.
|
|
||
| @Before | ||
| public void resetSystemProps() { | ||
| System.setProperty("pantheon.docker", "false"); |
There was a problem hiding this comment.
This appears to be setting a system property but never sets it back in an @After.
PR description
Provides a first group of changes to the CLI like renaming options and changing subcommands.
Doc was also updated.
Fixed Issue(s)
fixes Jira issues NC-2014, NC-2016, NC-2031, NC-2032, NC-2126 and NC-2127.