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

What to do in 0.24 #1433

Closed
16 of 22 tasks
HeroicKatora opened this issue Feb 13, 2021 · 9 comments · Fixed by #1656
Closed
16 of 22 tasks

What to do in 0.24 #1433

HeroicKatora opened this issue Feb 13, 2021 · 9 comments · Fixed by #1656

Comments

@HeroicKatora
Copy link
Member

HeroicKatora commented Feb 13, 2021

We have collected some amount of cruft that could be fixed in a new major version. There is no single issue that is a pressing concern, some have workaround, but I'd want to arrive at somewhat of a consensus or plan nevertheless as in total it is awkward. In particular the Rust compiler has moved forward quite a bit. I'd make a next branch like last time, which I believe worked out quite well.

The main issues that could be addressed:

Pure renames and restructing

Design issues

Implementation is breaking

Unanswered questions

  • When to do the major version, how to communicate this.
  • Which MSRV to choose? Do we want to choose one at all? The alternative could be a build.rs file that enables specific features for specific versions only but I'm somewhat concerned for the testability of such a design. Apart from const-generics, is there any particularly relevant Rust feature that we'd be willing to wait for?
  • Any overlooked use case?
  • We've also seen some influx of native dependencies (webp, avif). Is there some way in which we can improve our design here, that requires a major version—e.g. feature selection?
@fintelia
Copy link
Contributor

I think it is a good time to start thinking about a 0.24 and I agree with your list of items to consider. Under the first point we should probably also include removal of the deprecated submodules under image::math::* (#1434)

My sense is that the biggest items for this release would/should be iterating the APIs for Pixel, GenericImage and DynamicImage. The async and no_std points could also be quite impactful, but I'm not sure how far we'll be able to get on those in the near term.

@HeroicKatora
Copy link
Member Author

HeroicKatora commented Mar 10, 2021

The next branch has been created, and CI enabled. The MSRV limit has also been removed.

@okaneco
Copy link
Contributor

okaneco commented Mar 16, 2021

I'd like to see more use of Option and Result return types where feasible to avoid panicking and make interfaces more ergonomic for users. There are several public facing APIs with recoverable states such as get_pixel and get_pixel_mut, ImageBuffer functions that handle Option with expect (but from_vec returns an Option), and functions that assert_[eq]!, some of which do return ImageResult and some don't. More imageops functions should probably return Result or Option. I understand some of these choices may have been made to maintain backwards compatibility.

I know efforts were made for GenericImage::opt_pixel but that was reverted in #1245 and I don't see any issues for it other than #1243 discussing the name. I'd like to see access methods that don't panic in the next version of image. Should I make a new issue to discuss that or use an existing issue like #1243 or #1340?

Another thing I've seen in a TODO in buffer.rs was the 'static bound on Pixel and if that is still necessary there and in imageops.

@HeroicKatora
Copy link
Member Author

Feel free to use #1243 for it. And Ive edited your points into the initial comment.

@HeroicKatora HeroicKatora pinned this issue Sep 22, 2021
@HeroicKatora HeroicKatora changed the title When to do 0.24 What to do in 0.24 Oct 2, 2021
@HeroicKatora
Copy link
Member Author

Regarding the MSRV policy, how will we handle it and which version should we choose. The situation changed a bit compared to our last choice:

  • With Rust 1.56 we have the ability to annotate the supported Rust version in Cargo.toml, making it accessible to yet-to-be-developed tooling. In particular, an MSRV aware resolver is being planned.
  • Moving to Edition 2021 is likely a good idea sooner or later.
  • The evolution of const is ongoing. This doesn't necessarily affect us directly but it is pretty restrictive to place the same restriction on all dependencies. In particular there are some that can potentially benefit from calculating certain tables, initial state, etc at compile time (jpeg, various decompressors).
  • In this irlo thread was discussed using -Z minimal-versions to run the msrv tests. It does not break when new versions of dependencies are released but shouldn't be used in production either. We could also use specific lockfiles instead.
  • rustc versions across the Linux Distribution universe:
    • Debian Bullseye released with 1.48.0
    • Debian bookworm is on 1.50 (unstable is naturally on 1.56)
    • Ubuntu 20.04 is on 1.41
    • Ubuntu 21.10 (Impish Indri) is on 1.51, 21.04 (Hirsuite Hippo) is on 1.50

@okaneco
Copy link
Contributor

okaneco commented Nov 1, 2021

Under Implementation is breaking, as a sub-point to

[ ] optional get_pixel methods (and more clarity on assert vs. Result)

#1500 added functions that returned Option on the ImageBuffer struct in a non-breaking manner but it hasn't been in a patch release for 0.23. Breaking changes would occur if this function was added to traits like GenericImage/GenericImageView or the corresponding functions had their signatures changed.

@5225225
Copy link
Contributor

5225225 commented Nov 8, 2021

Should DynamicImage be made NonExhaustive?

Apparently it's not at the moment (Mentioned in #1552), but it looks like the kind of enum we'd want to extend without it being a breaking change.

@jplatte
Copy link

jplatte commented Nov 23, 2021

Are there plans to do another 0.23.x release? I'm mostly interested because of #1538.

@antonok-edm
Copy link

antonok-edm commented Jan 11, 2022

+1 on another 0.23.x release. Looks like there's been a bunch of additions since the last release; in particular I'd love to see #1624 supported in a version on crates.io (jaredforth/webp#7 (comment) for context).

bors bot added a commit to foresterre/sic that referenced this issue Feb 1, 2022
1068: Upgrade sic to Rust edition 2021 r=foresterre a=foresterre

With a new edition also comes a new MSRV, namely Rust 1.56, the first release which added support for edition 2021.

In addition, the new MSRV is necessary for image 0.24:

image-rs/image#1433
https://github.com/image-rs/image/blob/master/CHANGES.md

Co-authored-by: Martijn Gribnau <garm@ilumeo.com>
bors bot added a commit to foresterre/sic that referenced this issue Feb 1, 2022
1068: Upgrade sic to Rust edition 2021 r=foresterre a=foresterre

With a new edition also comes a new MSRV, namely Rust 1.56, the first release which added support for edition 2021.

In addition, the new MSRV is necessary for image 0.24:

image-rs/image#1433
https://github.com/image-rs/image/blob/master/CHANGES.md

Co-authored-by: Martijn Gribnau <garm@ilumeo.com>
@HeroicKatora HeroicKatora unpinned this issue Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants