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

Adding Timestamp decode/encode #351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

junderw
Copy link

@junderw junderw commented Aug 18, 2024

This is a baseline just to have something to work off of, very WIP.

  1. Should we perhaps just use DateTime from the chrono crate under a feature flag instead of creating a simple enum. (I've decided against this, it's too much of a hassle.)
  2. How should we go about including it in rmp-serde if at all? (Made an initial implementation, but kind of hacky, awaiting feedback)
  3. Checking for invalid values (ie. nanos above 999_999_999) should probably be done better. I am not too familiar with this library and how I should be propagating such errors. (Especially with serde.)
  4. Also, please advise on any formatting requests. (I turned off rustfmt)

Let me know if this isn't something you're interested it.

@junderw junderw marked this pull request as draft August 18, 2024 15:00
rmp/src/decode/mod.rs Outdated Show resolved Hide resolved
@junderw junderw changed the title WIP: Adding Timestamp decode/encode Adding Timestamp decode/encode Aug 26, 2024
@junderw junderw marked this pull request as ready for review August 26, 2024 15:48
@jcuffe
Copy link

jcuffe commented Nov 24, 2024

@junderw thank you for getting this started 🙏 I'm interested in using this with serde, but I don't understand either library well enough to know what code to add to rmp_serde so that I can use rmp::Timestamp in my structs. Would you be interested in collaborating on this?

@junderw
Copy link
Author

junderw commented Nov 27, 2024

@jcuffe I added a hackey TimestampSerde wrapper. It's a simple wrapper only for serde serialize and deserialize.

The content is pub, so you can just take the content out after it's deserialized.

I added a test for each way, you can look at the test to see how it works.

@jcuffe
Copy link

jcuffe commented Nov 27, 2024

Excellent! Thank you @junderw 😁

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