Support big endian headers in addition to little endian. #49
Conversation
| /// The encoding and serialization of message bodies is up to the application code, | ||
| /// and it not affected by this configuration parameter. | ||
| #[derive(Debug, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] | ||
| pub enum Endian { |
There was a problem hiding this comment.
How do you feel about adding NativeEndian?
There was a problem hiding this comment.
Bad: if both ends use NativeEndian on different platforms it could theoretically be different.
I'd rather be explicit what it is.
There was a problem hiding this comment.
On the other hand, it makes sense for peers using unix sockets.... Hmmm.
There was a problem hiding this comment.
We could have a constructor that instantiates to one of the two endiannesses..
There was a problem hiding this comment.
Unix sockets were the use case I had in mind.
We could even forgo Endian for unix transport and just use native endian by default.
There was a problem hiding this comment.
We could even forgo
Endianfor unix transport and just use native endian by default.
That's not that easy though. You can use the StreamTransport with Unix stream sockets too.
There was a problem hiding this comment.
Ah yeah good point.
Edit: My PR approvals stands :P
There was a problem hiding this comment.
Added NativeEndian anyway ^^
Decided to go with a runtime setting. I expect overhead to be negligible.
This is sadly a major change, but in the future we could add config fields with a minor change, since the structs are now marked as non-exhaustive.
But since its a major change now anyway, I didn't preserve backwards compatibility for
MessageHeader::encode/decodeeither.