Doc restructuring update cbor protobuf - #3221
Conversation
| [//]: # (title: CBOR format) | ||
| <primary-label ref="experimental-general"/> | ||
|
|
||
| Concise Binary Object Representation ([CBOR](https://datatracker.ietf.org/doc/html/rfc7049)) is a compact binary format based on JSON. |
There was a problem hiding this comment.
https://datatracker.ietf.org/doc/html/rfc8949 is a newer version
| [//]: # (title: CBOR format) | ||
| <primary-label ref="experimental-general"/> | ||
|
|
||
| Concise Binary Object Representation ([CBOR](https://datatracker.ietf.org/doc/html/rfc7049)) is a compact binary format based on JSON. |
There was a problem hiding this comment.
Official docs use "The underlying data model is an extended version of the JSON data model", so let's stick to it — use 'extends' json instead of 'based on' json
There was a problem hiding this comment.
Maybe phrase it as "...format extending the JSON data model". As it is the underlying data model that is extensible, not the actual wire format.
There was a problem hiding this comment.
🤔 I think we should go with this then (to avoid the gerund):
Concise Binary Object Representation (CBOR) is a compact binary format that extends the JSON data model.
| <primary-label ref="experimental-general"/> | ||
|
|
||
| Concise Binary Object Representation ([CBOR](https://datatracker.ietf.org/doc/html/rfc7049)) is a compact binary format based on JSON. | ||
| It supports a subset of [JSON features](configure-json-serialization.md) and produces binary output instead of text. |
There was a problem hiding this comment.
This is kinda confusing because you've meant what configuration flags are available in kotlinx-serialization-cbor, and you're not talking about format itself.
There was a problem hiding this comment.
Also, is it really a subset or just a set of options that have the same name (and hopefully the same semantics)?
There was a problem hiding this comment.
good point 🤔 yes, perhaps we could just delete this sentence. It feels confusing like this. — we cover ignoreUnknownKeys below so I don't think this is needed after all.
|
|
||
| The [`Cbor`](https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-cbor/kotlinx.serialization.cbor/-cbor/) class provides two main functions: | ||
|
|
||
| * [`encodeToByteArray()`](https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-core/kotlinx.serialization/encode-to-byte-array.html) serializes objects to a binary array. |
There was a problem hiding this comment.
'binary array' term is quite ambiguous, can we use 'byte array' instead?
| Let's look at an example where a `Project` object is serialized into a binary array and then deserialized back to its original form: | ||
|
|
||
| ```kotlin | ||
| // Imports declarations from the serialization library |
There was a problem hiding this comment.
we've discussed to drop this comment, didn't we?
|
|
||
| <!-- REMOVE AFTER REVIEW: Like with CBOR examples, I felt like we have shown enough times how it looks in ProtoBuf hex notation so I removed that part to keep it a bit more concise. --> | ||
|
|
||
| ## Encode empty lists in ProtoBuf |
There was a problem hiding this comment.
We fixed this some time ago (
), so this chapter can be dropped, I thinkThere was a problem hiding this comment.
nice! 👍 Yes, I agree, let's remove it.
| } | ||
| ``` | ||
|
|
||
| <!-- REMOVE AFTER REVIEW: I wanted to add an example here as well, please let me know if this works --> |
|
|
||
| 1. Define a class for the entire message. | ||
| 2. Add a property of the polymorphic type to the class. Annotate this property with [`@ProtoOneOf`](https://kotlinlang.org/api/kotlinx.serialization/kotlinx-serialization-protobuf/kotlinx.serialization.protobuf/-proto-one-of/). | ||
| 3. Create a `sealed interface` or `abstract class` to represent the fields inside the `oneof` declaration. |
There was a problem hiding this comment.
Why did you reorder 2 and 3? It is more confusing now. Also, it's better to stick to to the 'oneof group' rather than 'fields inside', because fields are represented as subclasses.
Also, why did you delete examples in-text? I think they're worth keeping. Even better, you can make the same structure you used in other docs:
- Create a
sealed interfaceorabstract classto represent the fields inside theoneofdeclaration.
@Serializable sealed interface IPhoneType
There was a problem hiding this comment.
the order is just to represent the order of the code example. 🤔 but yes, rereading, I think we should switch it.
make the same structure you used in other docs
Sure, I wanted to keep it more concise, but the same structure works as well 👍
|
|
||
| // Represents the work_phone field | ||
| @OptIn(ExperimentalSerializationApi::class) | ||
| @Serializable data class WorkPhone(@ProtoNumber(3) val number: String): IPhoneType |
There was a problem hiding this comment.
You lost explanation comment on why one of the classes is value and another is data. Why?
There was a problem hiding this comment.
nice catch! It's an important piece of info, so I re-added it to the step list to have this information, but in the example, it feels arbitrary to me.
| However, with Kotlin serialization, you can use Kotlin classes annotated with `@Serializable` as the source for the schema, making `.proto` files optional. | ||
|
|
||
| This approach simplifies the process when all the code involved is written in Kotlin, | ||
| interoperability with other languages often still requires a `.proto` schema. |
There was a problem hiding this comment.
| interoperability with other languages often still requires a `.proto` schema. | |
| but interoperability with other languages often still requires a `.proto` schema. |
|
Why base for this PR is |
3 participants
This PR adds the CBOR and ProtoBuf pages