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

Time doesn't implement Serialize and Deserialize #696

Open
dev-ardi opened this issue Mar 8, 2024 · 5 comments
Open

Time doesn't implement Serialize and Deserialize #696

dev-ardi opened this issue Mar 8, 2024 · 5 comments
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations

Comments

@dev-ardi
Copy link

dev-ardi commented Mar 8, 2024

I would expect time to convert pretty naturally to std::time::Duration but that's not the case

@epage
Copy link
Member

epage commented Mar 8, 2024

A Duration is serialized as seconds and nanoseconds and is meant to be a "time since" something

A Time is a "time of day" which is a different unit and would not be interchangeable. If we added serialize/deserialzie support, it would be to allow exclusively serializing/deserializing as just a Time and not a full DateTime.,

@dev-ardi
Copy link
Author

dev-ardi commented Mar 8, 2024

Right not Time has no serde implementation, which makes it impossible to use if you're using serde.
While it is a "time of day" it can be used as a duration too, if you interpret it that way!

execute_command_in = 00:10:10

can mean a duration indeed.

I'm not pushing for Duration specifically even if it makes sense for me, it was just an idea.
I'm fine if it deserializes into a toml::Value::Time, and then if it makes sense we can impl Into<Duration> for Time

@epage epage added C-enhancement Category: Raise on the bar on expectations A-serde Area: Serde integration labels Mar 8, 2024
@epage
Copy link
Member

epage commented Mar 8, 2024

I'm fine with serde support for those all of the date/time types but Duration I am against Duration for now.

@dev-ardi
Copy link
Author

dev-ardi commented Mar 8, 2024

Are you against deserializing into Duration or having an impl Into<Duration> for Time

@epage
Copy link
Member

epage commented Mar 8, 2024

Both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants