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

Fix invalid argument order #832

Merged
merged 4 commits into from
Feb 10, 2020
Merged

Conversation

cecton
Copy link
Contributor

@cecton cecton commented Feb 10, 2020

@bkchr this PR enforces this behavior:

[0] [11:24:02] ~/r/polkadot master > ./target/debug/polkadot purge-chain --dev -y
"/home/cecile/.local/share/polkadot/chains/dev/db" did not exist.
[0] [11:25:36] ~/r/polkadot cecton-fix-invalid-argument-order > ./target/debug/polkadot --dev purge-chain -y
error: Found argument 'purge-chain' which wasn't expected, or isn't valid in this context

USAGE:
    polkadot --dev

For more information try --help

Forked at: 80d7e03
Parent branch: master
```
--dev purge-chain
```

Must never be accepted
@parity-cla-bot
Copy link

It looks like @cecton signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@cecton cecton requested a review from bkchr February 10, 2020 11:27
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Please add a test.

Forked at: 80d7e03
Parent branch: master
Forked at: 80d7e03
Parent branch: master
@cecton cecton merged commit 1d21f77 into master Feb 10, 2020
@cecton cecton deleted the cecton-fix-invalid-argument-order branch February 10, 2020 12:21
cecton added a commit to paritytech/substrate that referenced this pull request Feb 21, 2020
…t be optional (#4842)

Related to #4776 
Related to paritytech/polkadot#832

To summarize the changes:
1. I did not manage to validate with types the service's Configuration. But I did reduce the possibility of errors by moving all the "fill" functions to their respective structopts
2. I split params.rs to multiple modules: one module params for just CLI parameters and one module commands for CLI subcommands (and RunCmd). Every command and params are in their own file so things are grouped better together and easier to remove
3. I removed the run and run_subcommand helpers as they are not helping much anymore. Running a command is always a set of 3 commands: 1. init 2. update config 3. run. This still allow the user to change the config before arguments get parsed or right after.
4. I added tests for all subcommands.
5. [deleted]

Overall the aim is to improve the situation with the Configuration and the optional parameters, add tests, make the API more consistent and simpler.
jordy25519 pushed a commit to plugblockchain/plug-blockchain that referenced this pull request Feb 27, 2020
…t be optional (#4842)

Related to #4776 
Related to paritytech/polkadot#832

To summarize the changes:
1. I did not manage to validate with types the service's Configuration. But I did reduce the possibility of errors by moving all the "fill" functions to their respective structopts
2. I split params.rs to multiple modules: one module params for just CLI parameters and one module commands for CLI subcommands (and RunCmd). Every command and params are in their own file so things are grouped better together and easier to remove
3. I removed the run and run_subcommand helpers as they are not helping much anymore. Running a command is always a set of 3 commands: 1. init 2. update config 3. run. This still allow the user to change the config before arguments get parsed or right after.
4. I added tests for all subcommands.
5. [deleted]

Overall the aim is to improve the situation with the Configuration and the optional parameters, add tests, make the API more consistent and simpler.
jordy25519 pushed a commit to plugblockchain/plug-blockchain that referenced this pull request Feb 28, 2020
…t be optional (#4842)

Related to #4776 
Related to paritytech/polkadot#832

To summarize the changes:
1. I did not manage to validate with types the service's Configuration. But I did reduce the possibility of errors by moving all the "fill" functions to their respective structopts
2. I split params.rs to multiple modules: one module params for just CLI parameters and one module commands for CLI subcommands (and RunCmd). Every command and params are in their own file so things are grouped better together and easier to remove
3. I removed the run and run_subcommand helpers as they are not helping much anymore. Running a command is always a set of 3 commands: 1. init 2. update config 3. run. This still allow the user to change the config before arguments get parsed or right after.
4. I added tests for all subcommands.
5. [deleted]

Overall the aim is to improve the situation with the Configuration and the optional parameters, add tests, make the API more consistent and simpler.
General-Beck pushed a commit to General-Beck/substrate that referenced this pull request Mar 4, 2020
…t be optional (paritytech#4842)

Related to paritytech#4776 
Related to paritytech/polkadot#832

To summarize the changes:
1. I did not manage to validate with types the service's Configuration. But I did reduce the possibility of errors by moving all the "fill" functions to their respective structopts
2. I split params.rs to multiple modules: one module params for just CLI parameters and one module commands for CLI subcommands (and RunCmd). Every command and params are in their own file so things are grouped better together and easier to remove
3. I removed the run and run_subcommand helpers as they are not helping much anymore. Running a command is always a set of 3 commands: 1. init 2. update config 3. run. This still allow the user to change the config before arguments get parsed or right after.
4. I added tests for all subcommands.
5. [deleted]

Overall the aim is to improve the situation with the Configuration and the optional parameters, add tests, make the API more consistent and simpler.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants