-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: migrate from structopt to clap #127
Conversation
90e9bfb
to
0bb97e9
Compare
e70c1be
to
39887eb
Compare
This task is blocked by #143. |
a326a89
to
d9d6815
Compare
Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
6e59e24
to
5ee5ec6
Compare
We need to specify verbatim_doc_comment to show multiple line doc comments for CLI properly. Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
5ee5ec6
to
aef0f59
Compare
Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
aef0f59
to
aef8006
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for tackling this hard task. I left a few comments. Could you please look them?
#[clap(long)] | ||
pub third_party_attribution: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and the current implementation do not cause any error even if we execute cargo run -- --shell --third-party-attribution ls
. I think the current implementation can be improved using ArgGroup. Should we scope this change into this PR or address in another pull request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will address this point in another pull request.
Signed-off-by: Ryota Sakamoto <skmt@amazon.com>
d4c2523
to
0a87300
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for checking my comment. LGTM
Issue #, if available:
close #109
Description of changes:
structopt is now in maintenance mode so we need to migrate into clap.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.