time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.13k stars 281 forks source link

Document status of local-offset support #687

Closed Aeledfyr closed 5 months ago

Aeledfyr commented 5 months ago

As of time 0.3.36, any method that requires the system's local offset will fail on multithreaded unix-like systems, but this is not mentioned in the documentation for the local-offset feature, or any of the relevant methods.

Places where this should hopefully be documented:

297 gives a good description of this problem; it was closed after adding documentation in https://github.com/time-rs/time/commit/288c942a34e19fa38704bdde5e1d49fc21318be2.

However, this documentation was removed in https://github.com/time-rs/time/commit/b513927af2e2a48b87a5374510abf9e703422d36, and replaced with "Without [unsound_local_offset], any method that requires the local offset will return the Err variant." in the crate-level documentation.

Later, https://github.com/time-rs/time/commit/110e17bb8461816551f7197a2aa2e34c981fa22d fully removed the reference to the local offset methods failing by default.

At this point, there is no documentation of the default behavior in places where people would look -- in the documentation of the local-offset feature or in the documentation of the methods that will fail, leading to a large number of issues asking about it: #591 #457 #297 #617 #538 #427 #645

I'm open to making a PR to add documentation, but it'll likely need to be carefully phrased.

jhpratt commented 5 months ago

This may be changing in the near future. Mutating the environment is unsafe starting in the 2024 edition (due to be released in October), and I expect that the associated lint will be warn-by-default (if not force-warn) in all editions ≤2021 soon. If and when that happens, I will revisit the issue to see if it remains an issue.

Given the inherent lack of a stability promise and the fact that it may change relatively soon, I think it would be best to not document it at this time. My comment here remains my position.

JensMertelmeyer commented 3 months ago

I think it would be best to not document it at this time.

I'm not a smart person, so it took me like half an hour to figure out what was going wrong. The documentation could have saved me 😭