Open thomaseizinger opened 1 year ago
Ah I was planning to remove the length prefix in serialize_into_vec()
too as per my comment in #202. I guess you guys would be ok with that too? I don't think we should necessarily retain the old function with the length prefix in any form either; so far no one seems to have voiced out in favor of that.
Cool!
Yeah I am okay with it being removed. That isn't the only part of my proposal though so I am keen to hear your opinion on the remaining points :)
Yep, I intend to submit a PR to remove the length prefixes soon.
Sorry I'm not very sure what you mean by duplication; could you briefly clarify?
Sorry for the late reply, this kind of went under the radar :sweat_smile:
As far as I know,
MessageInfo
is meant to be kinda optional (as in, users can choose to generateMessageInfo
implementations for autogenerated structs through a cmd line arg), so unifying all three might be a bit messy. So far, I don't see technical issues unifyingMessageRead
andMessageWrite
, though. Would that make things easier?
It is really about reducing the API surface for the user. All three traits are auto-generated, right? But all three have to imported when you want to interact with the generated code.
Hmm, I assume the focus here is on improving the user interface (as opposed to making the library codebase smaller).
100%. I think the main APIs exposed to the user should be as simple as possible and serve the majority of use cases. Ideally, they only have to deal with concepts they are already familiar with, like std::io::Write
.
We might then be able to make
Writer
,WriterBackend
andBytesWriter
private. We still needWriter
andWriterBackend
to exist since they serve to encapsulate protobuf-specific writing functions (i.e. write a varint, write with a tag, etc).
That would be ideal! :)
Not sure how ergonomic the no_std
use-case needs to be served but it is likely the less popular one so as long as it is possible, it should be good enough?
I'm not sure if I understand this suggestion correctly, but currently, the module root directly exposes:
It is mostly about following similar naming patterns and avoiding unnecessary "naming". If I am using quick_protobuf
, I already know that I'll be serializing / deserializing. The function name doesn't have to say that :)
I am a big fan of calling top-level module functions via their crate name. This adds a lot of context to the code without having to look up imports which makes reading and understanding code much easier because it doesn't interrupt your flow. For example:
fn main() {
let message = quick_protobuf::from_slice::<MyMessage>(&buf).unwrap();
}
Since a core focus of this library is on speed optimization, I would want to keep
serialize_into_slice()
available too, since it's an option that doesn't require memory allocation.
Sure! But again, we should drop the serialize_
prefix because it is redundant :)
We are in the process of adopting
quick-protobuf
inrust-libp2p
and a few points came up whilst using the API.MessageWrite::write_message
andWriter::write_message
. The latter uses the former but also includes a length-prefix. We've tripped over this multiple times.serialize_into_vec
which writes a length-prefix but it is not documented.BytesWriter
, aWriter
, aWriterBackend
and theMessageWrite
. The relationship is not entirely clear from the naming.My suggestions would be:
MessageInfo
,MessageRead
andMessageWrite
into a single traitMessage
.std::io::Write
. CanMessage::write
perhaps simply take a&mut std::io::Write
? This would remove the layer of constructing aWriter
with aWriterBackend
. Astd::fs::File
already implementsWrite
and so doesstd::vec::Vec
.AsyncMessage
type that uses thefutures
abstractionAsyncRead
andAsyncWrite
.quick_protobuf::from_slice::<M>(&[u8]) -> Result<M>
quick_protobuf::to_vec::<M>(M) -> Vec<u8>
serde
is a battle-tested library. We should be able to take some inspiration from what functions they provide: https://docs.rs/serde_json/1.0.93/serde_json/#functionscodegen
orinternal
which clearly signals to users that they shouldn't use these directly.Let me know what you think. I am open to working on this and sending PRs :)