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

Make BlockNumber in CLI generic as well #2653

Closed
bkchr opened this issue May 22, 2019 · 2 comments · Fixed by #4376
Closed

Make BlockNumber in CLI generic as well #2653

bkchr opened this issue May 22, 2019 · 2 comments · Fixed by #4376
Labels
I7-refactor Code needs refactoring. Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be.
Milestone

Comments

@bkchr
Copy link
Member

bkchr commented May 22, 2019

For some use cases in the CLI we require the BlockNumber, currently we assume the block number fits into u32. This is not correct and thus, we should make the CLI generic over the BlockNumber as well.

#2602

@bkchr bkchr added this to the As-and-when milestone May 22, 2019
@bkchr bkchr added the I7-refactor Code needs refactoring. label May 22, 2019
@bkchr bkchr added hacktoberfest Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be. labels Oct 1, 2019
@niklasad1
Copy link
Member

@bkchr

Do you mean that for example these should take a type parameter instead in u32?

It doesn't seem possible to do with structopt, see TeXitoi/structopt#128

@bkchr
Copy link
Member Author

bkchr commented Dec 11, 2019

Yeah we should remove the u32. We could create your own type struct BlockNumber(String) that we use as parameter to structopt. Parse or whatever structopt uses would make sure that the given parameter is really a number and not a string. In Substrate itself we just put a bound on the "real" BlockNumber type that it needs to implement From<String> or something similar. I think that should work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring. Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants