Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Add wrapper type to handle keys and values based on bincode serialization #805

Merged
merged 1 commit into from
Jun 16, 2024

Conversation

adamreichold
Copy link
Contributor

The first commit is just to make current Clippy build cleanly and can be drop if so desired.

Not sure if you want such additions as this could be considered "playing favourites" with a particular format and you might see requests to add many others. OTOH, Bincode is a pretty decent default for binary serialization formats of small messages.

@cberner
Copy link
Owner

cberner commented May 8, 2024

Hmm, ya I'm not sure I want to add this. My philosophy has been to leave out extra features which users can easily implement on their own, but I'll give it some more thought. If you want to send that first Clippy commit has a separate PR I'd be happy to merge that

@adamreichold
Copy link
Contributor Author

Hmm, ya I'm not sure I want to add this. My philosophy has been to leave out extra features which users can easily implement on their own, but I'll give it some more thought.

Maybe as one argument to why this could be useful upstream: While the code can easily be implemented downstream, there is also basically one way to write it and hence every downstream project will contain a copy of this same code creating quite a bit of unnecessary busy work. So I think shipping something like this could be good community service and make the project more approachable.

If you want to send that first Clippy commit has a separate PR I'd be happy to merge that

This is #807

@adamreichold adamreichold force-pushed the bincode-key-value branch 2 times, most recently from 8d5f237 to 596298b Compare May 10, 2024 06:06
@cberner
Copy link
Owner

cberner commented Jun 15, 2024

Sorry it took me a while to reply here. I thought about this some more and decided not to merge it now. If you'd like to submit it as a PR in the examples/ directory I'd be happy to put it there, and then people can copy if they want

@adamreichold
Copy link
Contributor Author

Understood. Will rework this into an example.

@adamreichold
Copy link
Contributor Author

Reworked into an example based on the existing int_keys one.

@cberner cberner merged commit 2036664 into cberner:master Jun 16, 2024
3 checks passed
@cberner
Copy link
Owner

cberner commented Jun 16, 2024

Merged, thanks!

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.

None yet

2 participants