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

Validate provided info in Encoder::with_info #442

Merged
merged 1 commit into from
Dec 25, 2023

Conversation

fintelia
Copy link
Contributor

This would normally be a breaking change, but the Encoder::with_info API hasn't yet been included in a release.

@hoijui
Copy link
Contributor

hoijui commented Dec 19, 2023

Good call!
I now imagine something like Info::validate(&self) -> Result<(), ...>, which is called from withing Encoder::with_info().
What I do not understand though, is how to prevent a lot of code duplication between this validate function
and all the set_* functions in Encoder, as the former would validate all at once, and the later validates parts separately.
Would we want to have separate Info::validate_* functions as well, and call the appropriate one from each Encoder::set_* method, and also call all of them in sequence from Info::validate()?

@hoijui
Copy link
Contributor

hoijui commented Dec 19, 2023

ouh you already did this in code.. sorry, did not see it is a PR ups.
... Is that already all the validation necessary?

@fintelia fintelia merged commit d761f16 into image-rs:master Dec 25, 2023
19 checks passed
@fintelia fintelia deleted the validate-info branch December 25, 2023 21:13
@fintelia
Copy link
Contributor Author

This is all the validation we were already doing in other cases. Not sure if any further validation makes sense

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