Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

cli overhaul #1600

Merged
merged 99 commits into from
Jul 25, 2016
Merged

cli overhaul #1600

merged 99 commits into from
Jul 25, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Jul 12, 2016

changes:

  • fixed incorrect parsing of decimal numbers
  • changed cache cli options and fixed their default values (cache will be working now)
  • fixed inconsistency in rpc modules started
  • removed die!, cause it's not testable
  • added notifications about deprecated cli options
  • removed unnecessary complexity in multiple places
  • tests, tests, tests
  • many (not all) command line params are now parsed to intermediate structures which validate them

debris added 30 commits July 5, 2016 17:52
@coveralls
Copy link

coveralls commented Jul 23, 2016

Coverage Status

Coverage increased (+0.5%) to 75.962% when pulling ad8e8a6 on cli_commands into 247495f on master.

@coveralls
Copy link

coveralls commented Jul 24, 2016

Coverage Status

Coverage increased (+0.6%) to 76.034% when pulling 286812d on cli_commands into 247495f on master.

@arkpar arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 24, 2016
@gavofyork gavofyork added A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jul 24, 2016
@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.6%) to 76.115% when pulling 0218c6e on cli_commands into 8574bfd on master.

@keorn keorn mentioned this pull request Jul 25, 2016
3 tasks
@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage increased (+0.4%) to 76.016% when pulling ce6f59e on cli_commands into 435ba18 on master.

@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A7-looksgoodcantmerge 🙄 Pull request is reviewed well, but cannot be merged due to conflicts. labels Jul 25, 2016
@@ -106,7 +107,7 @@ API and Console Options:
interface. APIS is a comma-delimited list of API
name. Possible name are web3, eth, net, personal,
ethcore, ethcore_set, traces.
[default: web3,eth,net,ethcore,personal,traces].
[default: web3,eth,net,ethcore,personal,traces,rpc].
Copy link
Contributor

Choose a reason for hiding this comment

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

rpc option not covered in above description

Copy link
Contributor

Choose a reason for hiding this comment

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

it's a meta-api, probably no reason to configure it in the cli, it should be just silently included
imo

Copy link
Contributor

Choose a reason for hiding this comment

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

then it shouldn't really be in the user-visible options

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Jul 25, 2016
@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Jul 25, 2016
@gavofyork
Copy link
Contributor

might be that there is a better solution than exposing rpc as an optional RPC.

@gavofyork gavofyork merged commit 226fe8e into master Jul 25, 2016
@gavofyork gavofyork deleted the cli_commands branch July 25, 2016 14:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants