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

Remove deprecated methods on Decoder #188

Merged
merged 1 commit into from
Nov 5, 2022

Conversation

fintelia
Copy link
Contributor

@fintelia fintelia commented Nov 5, 2022

Also removes current_chunk because it is no longer used elsewhere.

@fintelia fintelia mentioned this pull request Nov 5, 2022
Copy link
Member

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I think that solves your concern?

Not really. My concern is that the replacement that the deprecations recommend don't appear to be (ergnomomic) replacements to me. I'd be much gladder about the outright removal if we had a better migration story for downstream consumers.

src/decoder/mod.rs Show resolved Hide resolved
src/decoder/mod.rs Show resolved Hide resolved
@HeroicKatora
Copy link
Member

Not that we shouldn't do it regardless of migration, the breaking change does make sense because the current tracking of state isn't that well-designed either. In terms of maintenace, definitely better. I'd just be glad about a plan for replacing it.

@HeroicKatora
Copy link
Member

Quoting relevant information from the discussion:

It is useless for the user to get a chunk of image data without knowing which one it is, so they have to be tracking that information anyway.

So, if anyone happens to run across this PR as the 'cause' of breakage then know that we are open to design ideas that reflect the above in a well-design interface.

@HeroicKatora HeroicKatora merged commit 61234c0 into image-rs:master Nov 5, 2022
@fintelia fintelia deleted the decoder-deprecated branch November 5, 2022 20:36
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.

None yet

2 participants