apple / swift-protobuf

Plugin and runtime library for using protobuf with Swift
Apache License 2.0
4.55k stars 443 forks source link

error in readme #1692

Closed dongfanger2018 closed 1 week ago

dongfanger2018 commented 1 month ago

Before opening a new issue, please double check the past issues (both open and closed ones) to see if your problem has been discussed before and already contains an answer/solution. Likewise, check the FAQ in the Documentation folder for some common things - https://github.com/apple/swift-protobuf/blob/main/Documentation/FAQ.md

Please be sure to include:

Thank you!

// Serialize to binary protobuf format: you can choose to serialize into // any type conforming to SwiftProtobufContiguousBytes. For example: let binaryData: Data = try info.serializedBytes() let binaryDataAsBytes: [UInt8] = try info.serializedBytes()

should it be? let binaryData: Data = try info.serializedData()

thomasvl commented 1 month ago

serializedData still exists for compatibility, but the other apis are the preferred path forward.

@FranzBusch @gjcairo – Did you want to update the comments to reflect this? I guess all the parsing apis got annotated as deprecated, but not the serialization apis?

FranzBusch commented 1 week ago

IMO the docs are just wrong here serializedData should not be deprecated since it is totally fine to use and just a convenience overload at this point. cc @gjcairo

gjcairo commented 1 week ago

Apologies, I'd missed this issue.

I don't think the docs are wrong here. serializedBytes() is generic and returns some SwiftProtobufContiguousBytes.

serializedData() hasn't been deprecated and can still be used, but I don't think this particular bit of the docs should be changed.

However if you think it's worth mentioning, I can add something like:

Note that while the serializedBytes() spelling is generally preferred, you may also use serializedData() to get the bytes as an instance of Data where required.

gjcairo commented 1 week ago

https://github.com/apple/swift-protobuf/pull/1710