-
Notifications
You must be signed in to change notification settings - Fork 642
Use clap
for CLI argument parsing
#2993
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
Conversation
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.
Awesome! Just 1 copy paste bug noted below. Feel free to r=me.
Cargo.toml
Outdated
@@ -34,6 +34,7 @@ base64 = "0.13" | |||
cargo-registry-s3 = { path = "src/s3", version = "0.2.0" } | |||
chrono = { version = "0.4.0", features = ["serde"] } | |||
civet = "0.12.0-alpha.4" | |||
clap = "3.0.0-beta.2" |
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.
clap = "3.0.0-beta.2" | |
clap = "=3.0.0-beta.2" |
I'd pin the version since clap
often introduces breaking changes in beta releases (https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) and we could avoid updating it by mistake.
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.
Sounds reasonable. I've changed it to pinned. 👍
@bors r=jtgeibel |
📌 Commit c1bc624 has been approved by |
☀️ Test successful - checks-travis |
This should make it easier to convert these binaries to clap subcommands in a follow-up PR. It also simplifies our argument parsing code and generates help messages and proper errors if the passed arguments don't match our expectations.
r? @jtgeibel