search

tailhook / humantime

A parser and formatter for std::time::{SystemTime, Duration}
Apache License 2.0
283 stars 34 forks source link

Serde support as a cargo feature #26

Closed orhun closed 2 years ago

orhun commented 2 years ago

Currently, 2 dependencies are needed for using humantime and also de/serializing types:

humantime = "2.1.0"
humantime-serde = "1.1.1"

Wouldn't it be nice to unite these dependencies behind a feature gate? e.g.

humantime = { version = "2.1.0", features = ["serde"] }

This could be possibly done by adding a serde feature and exposing the humantime-serde crate if this feature is enabled. I'd be happy to submit a PR.

Or is it better to have these dependencies separated?

Any thoughts? @tailhook @jean-airoldie

jean-airoldie commented 2 years ago

Currently the humantime dependency is reexported via humantime-serde::re::humantime crate. So In this case you would only need 1 dependency.

As for your suggestion, it was discussed previously here.

The main problem is that compared to a crate implementing a new type with an optional serde feature, this is alternative serialization method to an std type that already implements serialize, which forces a bunch of additional types to be needed. So it feels more natural to have those types in a separate crate, if they are needed.