-
Notifications
You must be signed in to change notification settings - Fork 568
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
Conversation
unknownFields
type to [UInt32: Data]
from just Data
.unknownFields
type to [UInt32: Data]
from just Data
.
f4e2b45
to
33e8a5b
Compare
wire-runtime-swift/src/main/swift/ProtoCodable/ProtoReader.swift
Outdated
Show resolved
Hide resolved
wire-runtime-swift/src/main/swift/ProtoCodable/ProtoWriter.swift
Outdated
Show resolved
Hide resolved
wire-runtime-swift/src/main/swift/wellknowntypes/Duration.swift
Outdated
Show resolved
Hide resolved
33e8a5b
to
24c554b
Compare
24c554b
to
4a8f08e
Compare
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.
- Probably should get a second approval here
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.
Looks good as far as Wire the library is concerned.
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. |
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? |
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.