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

[swift] change unknownFields type to [UInt32: Data] from just Data. #2789

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

dnkoutso
Copy link
Collaborator

@dnkoutso dnkoutso commented Jan 19, 2024

This change improves the already difficult usability of unknown fields but also prepares for a follow up change to re-do Wire proto extensions for Swift.

This is a breaking change that will ship with 5.x of Wire which is a major version bump anyway.

@dnkoutso dnkoutso changed the title [wip] [swift] change unknownFields type to [UInt32: Data] from just Data. [swift] change unknownFields type to [UInt32: Data] from just Data. Jan 19, 2024
Base automatically changed from swift_visibility_cleanup to master January 19, 2024 16:47
@dnkoutso dnkoutso force-pushed the unknown_fields_dict branch 2 times, most recently from f4e2b45 to 33e8a5b Compare January 19, 2024 17:18
Copy link
Collaborator

@lickel lickel left a comment

Choose a reason for hiding this comment

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

  • Probably should get a second approval here

Copy link
Member

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

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

Looks good as far as Wire the library is concerned.

@swankjesse
Copy link
Member

Have ya seen how we track unknown fields in Java/Kotlin? We keep a ByteString with the encoded data for everything we couldn’t decode.

@dnkoutso
Copy link
Collaborator Author

I just did a quick check, certainly seems nicer to what we have in Swift right now. Is it more about the immutability, utility and performance to adopt something equivalent in Swift?

@dnkoutso dnkoutso merged commit eaff9ec into master Jan 21, 2024
7 checks passed
@dnkoutso dnkoutso deleted the unknown_fields_dict branch January 21, 2024 02:13
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

3 participants