-
-
Notifications
You must be signed in to change notification settings - Fork 103
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 structopt
to clap
#518
Conversation
I'm in favor. I already wanted to do this but I was just too lazy to pull through :) |
Still need to finish this off, one test fails. `structopt` was build on clap 2, and then its features were integrated into clap 3. So it's not actually a heavier dependency. May have to check on MSRV issues. Once we merge this, we only have one version of `syn`, which reduces our dependency size significantly.
precise-builds = true | ||
targets = ["aarch64-apple-darwin", "x86_64-apple-darwin", "x86_64-unknown-linux-gnu", "x86_64-unknown-linux-musl", "x86_64-pc-windows-msvc"] |
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.
Bleh this was some autoformatting; can revert if annoying
Need to upgrade structopt and then we'll reduce the compile time.
Can you have a look at the changes to the CI matrix here? |
OK, that should be fixed! |
@@ -11,21 +11,24 @@ keywords = ["snapshot", "testing", "jest"] | |||
categories = ["development-tools::cargo-plugins"] | |||
edition = "2018" | |||
readme = "README.md" | |||
rust-version = "1.61.0" | |||
rust-version = "1.64.0" |
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.
Note this bumps cargo-insta
to 1.64, which was released ~2 years ago, Sep 2022
insta
remains on 1.60.0
Still need to finish this off, one test fails.structopt
was build on clap 2, and then its features were integrated into clap 3 — ref TeXitoi/structopt#525. So it's not actually a heavier dependency. May have to check on MSRV issues.Once we merge this, we only have one version of
syn
, which reduces our dependency size significantly.This is ready for any feedback / testing. Will need to adjust the dependency spec if this gets in before we increase the
cargo-insta
msrv to 1.64...cargo.lock
, usecargo-msrv
to verify MSRV #524 for CI to pass, at least without manually pinning dependencies