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
Update LottieConfiguration to use DecodingStrategy. dictionaryBased by default #1777
Update LottieConfiguration to use DecodingStrategy. dictionaryBased by default #1777
Conversation
/// Use Codable. This is the default strategy introduced on Lottie 3. | ||
/// Use Codable. This is was the default strategy introduced on Lottie 3, but should be rarely | ||
/// used as it's slower than `dictionaryBased`. Kept here for any possible compability issues | ||
/// that may come up, but consider it soft-deprecated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is calling it soft-deprecated
too strong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect we'll never completely remove the Codable
conformances on our models, since it seems reasonable to continue providing a Codable
conformance for convenience (and since removing this would be source-breaking).
I do think it makes sense to call this case "legacy" or "deprecated" though, since it's no longer recommended. I think we may as well change it to case legacyCodable
now. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could still have the Codable
conformance, but eventually remove this convenience around using it. I'll rename it to legacyCodable
for now.
Co-authored-by: Cal Stephens <cal@calstephens.tech>
Co-authored-by: Cal Stephens <cal@calstephens.tech>
See #1776