Ogeon / palette

A Rust library for linear color calculations and conversion
Apache License 2.0
749 stars 60 forks source link

Updating to clap version 4. #364

Closed plugwash closed 7 months ago

plugwash commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

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.