search

image-rs / image-png

PNG decoding and encoding library in pure Rust
https://docs.rs/png
Apache License 2.0
347 stars 139 forks source link

Update the `bitflags` dependency to `2.x.x`. #469

Closed anforowicz closed 4 months ago

anforowicz commented 4 months ago

This PR will help to ensure that in my dependency graph there is only a single version of bitflags. Having v1 and v2 is not the end of the world, but it seems slightly nicer to be able to just depend on v2 of bitflags.

Also - after sending this PR out, I've realized that removing Ord and PartialOrd may make some sense from purity perspective, but having these traits is useful (e.g. if a client of the png crate ever wanted to put Transformations in BTreeSet) and avoids having to (somewhat unnecessarily) bump up the semver version of the png crate. I'll go ahead and try to fix this.

anforowicz commented 4 months ago

Also - after sending this PR out, I've realized that removing Ord and PartialOrd may make some sense from purity perspective, but having these traits is useful (e.g. if a client of the png crate ever wanted to put Transformations in BTreeSet) and avoids having to (somewhat unnecessarily) bump up the semver version of the png crate. I'll go ahead and try to fix this.

This is done now.

anforowicz commented 4 months ago

@fintelia, can you PTAL?

fintelia commented 4 months ago

This looks good to me, but has some overlap with #400.

Hoping to start working on a png 0.18 release in the next few weeks once image 0.25 is released.

anforowicz commented 4 months ago

This looks good to me, but has some overlap with #400.

Ack. Sorry for not realizing this earlier. I guess I can close/abandon this PR in this case (the doc comment changes are not that important I guess).

Hoping to start working on a png 0.18 release in the next few weeks once image 0.25 is released.

Ack.

I didn't realize that bumping to 2.x.x of bitflags is a breaking API change but now I see that Transformations::from_bits_unchecked has indeed disappeared after this PR. Thanks for being on top of that.