+
Skip to content

Conversation

MarkLindblad
Copy link
Contributor

No description provided.

@MarkLindblad MarkLindblad requested review from alexaryn and bsowell July 6, 2025 21:09
Copy link
Collaborator

@alexaryn alexaryn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see our serialization format start with a magic number and version. I'm thinking 8 bytes like: Aryn major16 minor16.

Overall, this seems like a sane way to use MessagePack, but I don't know enough about MessagePack to know if it'll satisfy our eventual needs.

@MarkLindblad
Copy link
Contributor Author

I'd like to see our serialization format start with a magic number and version. I'm thinking 8 bytes like: Aryn major16 minor16.

Overall, this seems like a sane way to use MessagePack, but I don't know enough about MessagePack to know if it'll satisfy our eventual needs.

I've now added magic bytes, a version header, and zero padding.

Copy link
Collaborator

@alexaryn alexaryn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we need to raise something other than ValueError in most cases. Generally, I think of ValueError as more of a programming error. I see agreement here: https://stackoverflow.com/questions/79590536/what-python-exception-should-i-raise-when-an-input-file-has-the-wrong-size

@MarkLindblad
Copy link
Contributor Author

I still think we need to raise something other than ValueError in most cases. Generally, I think of ValueError as more of a programming error. I see agreement here: https://stackoverflow.com/questions/79590536/what-python-exception-should-i-raise-when-an-input-file-has-the-wrong-size

Done.

Copy link
Collaborator

@alexaryn alexaryn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great.

@MarkLindblad MarkLindblad merged commit 7edad90 into main Jul 21, 2025
18 of 23 checks passed
@MarkLindblad MarkLindblad deleted the mark/serialize branch July 21, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载