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

Rename Reader to ImageReader #2243

Merged
merged 4 commits into from
May 27, 2024

Conversation

ripytide
Copy link
Member

I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to choose either at their option.

Following the discussion from #1327

@fintelia fintelia mentioned this pull request May 26, 2024
7 tasks
@fintelia
Copy link
Contributor

Created #2245 to track issues like this that'll need a breaking release

@kornelski kornelski merged commit 6d5918f into image-rs:main May 27, 2024
31 checks passed
@kornelski
Copy link
Contributor

I think it'd be helpful to create a pub use under old name with a deprecated attribute. It makes migration less painful.

@fintelia
Copy link
Contributor

I'm not sure you can place a deprecated attribute on a re-export.

But in either case, in the future @kornelski let's avoid merging breaking changes directly to the main branch. I'd prefer to keep the main branch in a state where it is always possible to make a patch release if needed. We can discuss major release strategy more in the linked issue, but I wasn't planning to start it for another 6+ months.

fintelia added a commit that referenced this pull request May 27, 2024
@ripytide ripytide deleted the rename-reader-to-imagereader branch May 27, 2024 09:12
@kornelski
Copy link
Contributor

kornelski commented May 27, 2024

Deprecated attribute works fine on re-exports and correctly fires only when the reexported name is used.

The attribute needs to be on pub type. Still doable.

@ripytide ripytide restored the rename-reader-to-imagereader branch May 27, 2024 11:21
@fintelia
Copy link
Contributor

fintelia commented Jun 2, 2024

@ripytide could you open a PR with the necessary re-export?

I'd also like to give @HeroicKatora a chance to weigh in. They had some concerns with the renaming before, and while they've been less active recently they are still a maintainer.

@HeroicKatora
Copy link
Member

The previous design is to large parts modelled around the standard library, that is small modules and avoid prefix duplication (the library is image, it shouldn't preface everything with another Image*). That decision was before experience of wgpu has demonstrated, to me but also in general, that the inverse is also possible and a large top level is still a workable library usage experience. I'd prefer to keep the io module itself for uncommon types related specifically to it but I don't have any deeper concerns about the renaming.

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 this pull request may close these issues.

4 participants