time
time copied to clipboard
Breaking change wish-list
Note: There is no breaking change current planned. This issue is to keep track of things that may happen when there is a breaking change at some undetermined point in the future. Items are in no particular order.
Potential changes
These are mostly things that I have thought about at some point, with varying levels of certainty.
- Eliminate
large-dates
feature flag. Support the ±999,999 range of years unconditionally, adding a modifier to the[year]
component to solve the issue of ambiguity. - Remove the
serde-well-known
feature flag, which is already deprecated in favor of using the relevant flags (serde
,formatting
, and/orparsing
) directly. - Simplify error handling. Anonymous enums would be wonderful but are not required. In particular, the large number of conversions should be cut down, as many of them exist when they can already be performed transitively. The amount of information exposed should be restricted to as little as necessary, as this has caused issues in the past.
- Remove anything deprecated. This is currently minimal.
- Make
FormatItem
opaque and requireFormatItem::Literal
to be valid UTF-8. Likewise forOwnedFormatItem
. - Remove
Copy
implementation forParsed
. - Use ranged integers in public APIs. This would currently require adding countless new methods.
- Rename
Duration
toSignedDuration
or something else. - Make
Duration::seconds
generic overi64
,f32
, andf64
. Similarly forsaturaturating_seconds_*
andchecked_seconds_*
. Alternatively, have aseconds_float
method that is generic while keeping the integer case separate. - Remove
Duration::time_fn
. This is a carry-over fromtime
0.1 and was presumably meant as a poor man's benchmarking tool.Instant
is not meant to be used in that manner. - Remove
time::Instant
in favor of an extension trait onstd::time::Instant
. This would reduce the number of trait implementations needed. The extension trait was added in v0.3.35, withtime::Instant
being deprecated at the same time. - Semantically permit modifiers on
[optional]
and[first]
(needed for #708)
Separate feature flags for serde
and the provided impl Serialize/Deserialize for T
would be very helpful (and address https://github.com/time-rs/time/issues/672)
@jeff-hiner I'll be honest, it's not likely to happen. The clippy issue you linked would solve your request while being more general purpose and would avoid the overhead of an additional feature flag. I'm also not aware of any widely-used crates that gate serde
support and Serialize
/Deserialize
impls separately.
I'm also not aware of any widely-used crates that gate
serde
support andSerialize
/Deserialize
impls separately.
The url
crate (167M downloads) implement the serde
traits with a sensible representation (the string representation one expects), and then provides serialize_internal
and deserialize_internal
for when you want to use a more performant serialization format.
To me the situation here is similar to Path
not implementing Display
, but still providing the Path::display
method that returns something that implements Display
, because providing the implementation directly would be a footgun.
Note that I was responding to the use of separate feature flags for Serialize
/Deserialize
impls, which is not present in url
. Regardless, time
also uses a sensible representation for implementing serde
traits — the same format as is used for Display
. Just as serialize_internal
and deserialize_internal
necessitate the use of serde
helper attributes, you can already use helper attributes provided by time
on the relevant fields. The only concern I have seen raised is relating to the default not being a well-known format, but that can be worked around if and when that clippy issue is resolved. If it's that much of an issue for you, I suggest you bring it up with your (common) employer to find some time to work on it.
Path
not implementing Display
is not because it's a footgun but because it's a lossy conversion. That is not the case for any type in time
.
Regardless,
time
also uses a sensible representation for implementingserde
traits — the same format as is used forDisplay
.
That's false. You find in the string printed by the Display
the same amount of information as in the tuple the serde
impl serializes to, but the serde
impl doesn't serializes as a string. With this logic, you could argue that url's serialize_internal
uses the same format as Display
, and that would be wrong.
Oh with the human-readable serializer you mean? That's fair.
@jeff-hiner I'll be honest, it's not likely to happen. The clippy issue you linked would solve your request while being more general purpose and would avoid the overhead of an additional feature flag.
Looks like the clippy issue has been parked for 2 years. It was claimed, but I don't see any progress over the last year. I'd try to PR in a fix myself but as it involves digging into clippy's parser I honestly have no idea where to even start. Not optimistic that's going to be fixed soon.
I'm guessing here, but it looks like the current default is intended for human-readable serialization like serde_yaml
. I suppose the similarity with Display
is convenient for that use case. Unfortunately this default is unsuitable for those of us using serde_json
and time
to write REST endpoints or API clients, because it doesn't appear to follow any RFC or ISO convention.
I don't mind having to explicitly annotate serde(with = "...")
. But the compiler can't help me if I miss one, because there's a valid but incompatible impl in scope. chrono::DateTime<Utc>
defaults to RFC3339, which is why I'm only noticing it on my migration.
Since there's no one "right" way to impl Serialize for OffsetDateTime
, maybe we simply shouldn't provide one. Removing the default Serialize
/Deserialize
entirely and making users opt into a specific serde
impl set seems like the only sensible compromise. This would be a breaking change, thus why I'm bringing it up here.
It was claimed, but I don't see any progress over the last year.
Are we looking at the same thing? There was an update just a couple days ago on a PR that implements it. It may have stalled, but it is at least on the person's backlog.
it looks like the current default is intended for human-readable serialization
The default is intended to be something human-readable (when that flag is enabled) that can also be deserialized. There was no reasoning beyond that.
Unfortunately this default is unsuitable for those of us using
serde_json
andtime
to write REST endpoints or API clients, because it doesn't appear to follow any RFC or ISO convention.
chrono::DateTime<Utc>
defaults to RFC3339
A trivial check shows that chrono
does not use RFC3339, as that specification does not support years that are negative or contain more than four digits.
Since there's no one "right" way to
impl Serialize for OffsetDateTime
, maybe we simply shouldn't provide one.
That's an awfully large leap. As I said, the intent was to have something that is human-readable and can be round-tripped. Just as with formatting being present because Display
is not everyone's cup of soup, there is a way to override the default. That there is not currently a way to prevent you from using the default does not mean we should take something reasonable away from those who want it. It's an ecosystem problem, not a time
problem imo.
It was claimed, but I don't see any progress over the last year.
Are we looking at the same thing? There was an update just a couple days ago on a PR that implements it. It may have stalled, but it is at least on the person's backlog.
I'm looking at https://github.com/rust-lang/rust-clippy/issues/8581 as linked in my original comment. That issue was opened 24 Mar 2022 (over two years ago), then claimed in June of the same year. A separate author opened a PR that was last touched at the beginning of the year, in Jan 2024. That PR received feedback in Feb, but no commits have been made since. As of right now the PR has conflicts. It appears the work required to have clippy support the appropriate parsing is non-trivial, or at the very least the PR implementation is controversial.
A trivial check shows that chrono does not use RFC3339, as that specification does not support years that are negative or contain more than four digits.
A trivial check on docs.rs shows that chrono does in fact use RFC3339 for DateTime<Tz>
. It's very possible the implementation isn't complete or correct, but RFC3339 appears to be the intent of the default serialization.
Things aren't fast in Rust given that it's entirely driven by volunteers. The PR author commented just this week that they intend to continue work on it. Given that you presumably are using this as part of your employment, that is why I suggested asking for some time to work on the issue.
It's very possible the implementation isn't complete or correct, but RFC3339 appears to be the intent of the default serialization.
It definitely isn't correct for the reason that I stated. It's not RFC3339 if it doesn't strictly follow the specification.
Regardless of the specific format chosen, a breaking change I would really like is for the time
crate to adopt some standard datetime representation as the default or simply get rid of the default Serialize
/Deserialize
impls. I would vote for ISO 8601, which avoids the RFC 3339 restriction on negative years.
The current default Serialize
impl is really frustrating because the format appears to be bespoke and is thus only meaningfully compatible with time
itself.
Removing the defaults is almost certainly not going to happen. ISO 8601 is at least a possibility, though keeping the non-human-readable compatibility will almost certainly happen.
thus only meaningfully compatible with time itself.
That's pretty much the point, actually. It's meant for when someone wants to serialize it for storage/transmission and then deserialize it at the other end. It is not and was never intended to be general-purpose — that's what the attributes are for.
I don't really mind the non-human readable variant, since as far as I know there is no widely used binary encoding standard for dates and times. But I think replacing the default human readable representation with a standardized one has only benefits: it avoids the work of maintaining an extra format and interop with other systems is easier out of the box.
For what it's worth, "maintaining" the extra format is essentially zero work, as it was a one-off thing that hasn't changed in the slightest. Changing it to a different format wouldn't be much work, and looking at the current implementation it's not even that different from ISO 8601.