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

Updating to clap version 4. #364

Closed
plugwash opened this issue Dec 19, 2023 · 7 comments · Fixed by #370
Closed

Updating to clap version 4. #364

plugwash opened this issue Dec 19, 2023 · 7 comments · Fixed by #370

Comments

@plugwash
Copy link

Hi.

I am one of the Debian rust team and we would like to reduce the number of clap versions we are shipping in Debian. Is there any chance of an update to use clap version 4?

@Ogeon
Copy link
Owner

Ogeon commented Dec 19, 2023

Hi, palette uses clap for examples only, so there's no hard dependency on any specific version. Do you still have to package the dev dependencies? 👀

The blocker here has been the minimum supported rust version, as I remember it, but I suppose it should be possible to move examples to their own crate. 🤔

@Ogeon
Copy link
Owner

Ogeon commented Dec 19, 2023

I should also say that I'm not going to be able to do a lot during at least the next day or two, for life and holiday reasons, so I hope this is not too urgent.

@plugwash
Copy link
Author

Hi, palette uses clap for examples only

That is good to know

Do you still have to package the dev dependencies? 👀

We do if we want to run tests, which while not 100% essential is considered highly desirable.

That said, we do have the option of patching stuff, either to update it ourselves or to remove the dev-dependencies and disable the stuff that depends on them.

I just did a test build and it looks like the examples do build succesfully with clap version 4.

The blocker here has been the minimum supported rust version,

Understandable.

Some crates have a different MSRV for tests than for the crate itself, but I can see how that makes testing more complex.

but I suppose it should be possible to move examples to their own crate. 🤔

That seems like overkill.

@Ogeon
Copy link
Owner

Ogeon commented Dec 19, 2023

I see, that makes sense. And yeah, the CI isn't set up for separate MSRV right now, so that would still take a bit of time to fix. The example that uses it is kind of bad anyway, so I wouldn't mind removing it, honestly. I have considered it before. Would it work for you to patch it on your side in the short term? I should be able to have the situation resolved for the next release, one way or the other.

@jonassmedegaard
Copy link

As I see it, a fix would be to extend (not bump) dependency on clap, like this:

--- a/palette/Cargo.toml
+++ b/palette/Cargo.toml
@@ -85,7 +85,7 @@
 scad = "1.2.2" # For regression testing #283
 
 [dev-dependencies.clap]
-version = "3.2.25"
+version = ">= 3.2.25, <= 4"
 default-features = false
 features = ["std"] # Required since 3.2.25
 

@Ogeon
Copy link
Owner

Ogeon commented Dec 22, 2023

That could also be an option, depending on how the CI setup handles it. It doesn't currently force the lowest allowed version of dependencies, but that's something that should be done as well.

@Ogeon
Copy link
Owner

Ogeon commented Jan 14, 2024

I ended up removing it, so the next release will be without any clap dependency. That allowed me to unpin a couple of other dependencies as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants