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

use clap3 instead of structopt #10632

Merged
merged 14 commits into from
Jan 24, 2022
Merged

use clap3 instead of structopt #10632

merged 14 commits into from
Jan 24, 2022

Conversation

koushiro
Copy link
Contributor

@koushiro koushiro commented Jan 11, 2022 •
edited
Loading

Description

use clap v3 instead of structopt (Close #10651)

waiting for a new version (includes the paritytech/ss58-registry#67) of ss58-registry to be released firstly

polkadot companion: paritytech/polkadot#4689
cumulus companion: paritytech/cumulus#895

Polkadot address: 15XuanNimo5951s3RjFTPX1AvYVnCwfr3SDhb3AV4fQF3LpK

@koushiro
use clap3 instead of structopt
8426046 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
format
c5ac613 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro koushiro mentioned this pull request Jan 11, 2022
Merged
@koushiro koushiro marked this pull request as ready for review January 11, 2022 06:10
@koushiro koushiro requested review from kianenigma and a team as code owners January 11, 2022 06:10
bkchr
bkchr reviewed Jan 11, 2022
View reviewed changes
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.

Ty for doing this. I just skimmed it, but it looks good!

bin/node/cli/src/command.rs Outdated Show resolved Hide resolved
bin/node/cli/src/command.rs Outdated Show resolved Hide resolved
@koushiro
update ss58-registry and revert some nits
badba9b 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Fix clippy and doc
0d23398 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro koushiro mentioned this pull request Jan 12, 2022
Merged
bkchr
bkchr approved these changes Jan 17, 2022
View reviewed changes
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.

Ty!

The amount of changes looks big, but as we already touch all these lines or most of them we can directly simplify all the code.

And also, please stop using force pushes.

bin/node-template/node/src/cli.rs Outdated Show resolved Hide resolved
bin/node/cli/src/cli.rs Show resolved Hide resolved
client/cli/src/commands/build_spec_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/build_spec_cmd.rs Outdated Show resolved Hide resolved
utils/frame/generate-bags/node-runtime/src/main.rs Outdated Show resolved Hide resolved
client/cli/src/commands/run_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/import_blocks_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/export_blocks_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/export_blocks_cmd.rs Outdated Show resolved Hide resolved
client/cli/src/commands/check_block_cmd.rs Outdated Show resolved Hide resolved
@koushiro
Apply review suggestions
f32465d 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
KiChjang
KiChjang reviewed Jan 17, 2022
View reviewed changes
client/cli/src/params/database_params.rs
case_insensitive = true,
possible_values = &Database::variants(),
ignore_case = true,
possible_values = Database::variants(),
Copy link
Contributor

Choose a reason for hiding this comment

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

No replacement to arg_enum here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if I use arg_enum here, do I need to remove the variants method of Database and impl std::str::FromStr for Database?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea what that means, I'm just pointing out how it looks different from the other enums where you put arg_num.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the variants method of other enums are generated by structopt, but the Database::variants and WasmExecutionMethod::variants are implemented manually.

@koushiro koushiro requested review from KiChjang and removed request for a team January 19, 2022 11:38
@bkchr
Copy link
Member

bkchr commented Jan 20, 2022

@koushiro CI is still not happy.

@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jan 20, 2022
@koushiro
cargo fmt
38f7b7c 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@bkchr
Copy link
Member

bkchr commented Jan 20, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Checks failed for 38f7b7c

@koushiro
Copy link
Contributor Author

@bkchr could you merge this PR? I don't want to resolve the conflicts again :(

@KiChjang
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 511e8c8 into paritytech:master Jan 24, 2022
@bkchr
Copy link
Member

bkchr commented Jan 24, 2022

/tip small

@substrate-tip-bot
Copy link

A small tip was successfully submitted for koushiro (15XuanNimo5951s3RjFTPX1AvYVnCwfr3SDhb3AV4fQF3LpK on polkadot).

https://polkadot.js.org/apps/#/treasury/tips

@koushiro koushiro deleted the use-clap3 branch January 25, 2022 09:42
eskimor pushed a commit that referenced this pull request Jan 27, 2022
@koushiro @eskimor
use clap3 instead of structopt (#10632)
e5567f0 
* use clap3 instead of structopt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* format

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update ss58-registry and revert some nits

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix clippy and doc

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update clap to 3.0.7

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Apply review suggestions

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* remove useless option long name

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* cargo fmt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@haerdib haerdib mentioned this pull request Feb 3, 2022
Closed
Wizdave97 pushed a commit to ComposableFi/substrate that referenced this pull request Feb 4, 2022
@koushiro @Wizdave97
use clap3 instead of structopt (paritytech#10632)
c23c599 
* use clap3 instead of structopt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* format

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update ss58-registry and revert some nits

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix clippy and doc

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update clap to 3.0.7

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Apply review suggestions

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* remove useless option long name

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* cargo fmt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@nanocryk nanocryk mentioned this pull request Feb 25, 2022
Merged
wischli added a commit to KILTprotocol/kilt-node that referenced this pull request Feb 25, 2022
@wischli
fix: use clap3 instead of structop
17a4986 
paritytech/substrate#10632
wischli added a commit to KILTprotocol/kilt-node that referenced this pull request Feb 28, 2022
@wischli
feat: upgrade to polkadot v0.9.17 (#330)
a26fd67 
* chore: bump deps to Polkadot v0.9.17

* fix: rm Default requirement for AccountId types

* fix: use clap3 instead of structop

paritytech/substrate#10632

* fix: AccountId doesnt impl Default anymore

* chore: bump deps

* chore: bump clap

* fix: clippy

* fix: EnsureDidOrigin

* fix: remove unwanted dependency leftover
@sander2 sander2 mentioned this pull request Mar 1, 2022
Merged
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
@koushiro @grishasobol
use clap3 instead of structopt (paritytech#10632)
a581d18 
* use clap3 instead of structopt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* format

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update ss58-registry and revert some nits

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix clippy and doc

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update clap to 3.0.7

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Apply review suggestions

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* remove useless option long name

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* cargo fmt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@koushiro @ark0f
use clap3 instead of structopt (paritytech#10632)
7f7839b 
* use clap3 instead of structopt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* format

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update ss58-registry and revert some nits

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix clippy and doc

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* update clap to 3.0.7

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Apply review suggestions

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* remove useless option long name

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* cargo fmt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@KiChjang KiChjang KiChjang approved these changes

@kianenigma kianenigma Awaiting requested review from kianenigma

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@koushiro @bkchr @KiChjang