-
Notifications
You must be signed in to change notification settings - Fork 152
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
Upgrade to clap 3. #1142
Upgrade to clap 3. #1142
Conversation
This requires moving away from the deprecated `clap_app!` macro. Clap's derive API (formerly `StructOpt`) is nicer anyway, because it's not stringly-typed. There is quite a bit of furniture moving in `execute.rs` to better separate the perf tools used for `bench_*` subcommands from those used for the profile subcommands.
9ae7744
to
daf36a4
Compare
@epage: this may be of interest after all my questions today, and I'd be happy to hear any suggestions you have. Thanks! |
Would you be up for bumping clap in database as well? That should be significantly easier (the options are much more limited, I suspect) -- it would be nice to avoid depending on 2.x and 3.x at the same time. |
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 didn't dig into verifying clap_app -> derive but instead focused on how the derive is being used.
No problem. I didn't even know that usage was there. I'll do it in a follow-up PR to avoid scope creep here. |
Thanks @epage for the helpful suggestions :) |
@nnethercote I think this broadly seems good to me, but it looks like there are some ongoing(?) conversations on potential cleanups/simplifications that could be done -- can you comment when you want to move ahead? |
@Mark-Simulacrum I'm happy with the current code as it is. |
Pretty minor stuff, mostly renamed functions.
Having said that, I just added a commit to update |
Note: The same "Test profiling" failure is happening on some other PRs, so it isn't caused by this PR's changes. |
b23676c
to
cc9e711
Compare
This requires moving away from the deprecated
clap_app!
macro. Clap'sderive API (formerly
StructOpt
) is nicer anyway, because it's notstringly-typed.
There is quite a bit of furniture moving in
execute.rs
to betterseparate the perf tools used for
bench_*
subcommands from those usedfor the profile subcommands.