Skip to content
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

Fixes #235 - Replace unmaintained structopt crate by clap #239

Closed
wants to merge 4 commits into from

Conversation

palant
Copy link
Contributor

@palant palant commented May 10, 2024

See #235 for the rationale. The change itself is pretty straightforward, the only issue here: existing code will have to be adapted (typically a trivial structopt::StructOpt => clap::Parser and Opt::from_args => Opt::parse search&replace).

A bunch of tests are failing for me, but that’s unchanged from current main branch.

@palant palant mentioned this pull request May 10, 2024
@palant
Copy link
Contributor Author

palant commented May 10, 2024

Concerning the clippy warning I fixed: that’s unrelated to my change. The only reason this issue wasn’t flagged on main is that the check was running Rust 1.77.2 there, and this warning is new in Rust 1.78.

@eaufavor eaufavor added the dependencies Pull requests that update a dependency file label May 10, 2024
@johnhurt johnhurt added the Accepted This change is accepted by us and merged to our internal repo label May 21, 2024
@johnhurt
Copy link
Contributor

Awesome. We merged this change internally, but had to go to an clap 3.x to maintain our minimum rust version. It will show up in the next internal -> external sync. Thanks!

drcaramelsyrup pushed a commit that referenced this pull request May 24, 2024
---
Fixed formatting

Includes-commit: 05f9754
Includes-commit: 29286c7
Replicated-from: #239
drcaramelsyrup pushed a commit that referenced this pull request May 24, 2024
---
Fixed formatting

Includes-commit: 05f9754
Includes-commit: 29286c7
Replicated-from: #239
@drcaramelsyrup
Copy link
Contributor

Thanks for contributing! This was merged into main in 795594d (aforementioned adjustment in 9dee8e6).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted This change is accepted by us and merged to our internal repo dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants