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

Implement find_local_time_type_from_local #683

Closed
wants to merge 16 commits into from

Conversation

esheppa
Copy link
Collaborator

@esheppa esheppa commented May 1, 2022

  • Implement find_local_time_type_from_local which uses a local instead of UTC timestamp to compare to the transitions and extra rules
  • All transitions are converted to a local timestamp by adding the relevant offset, prior to comparison with the desired local timestamp
  • Pass a LocalResult as far up as possible to ensure that ambiguous and skipped local timestamps are recognized

Potential additions:

  • Having an extra function try_from_local_datetime in the TimeZone trait which returns a LocalResult allowing users of the library to handle ambiguous or skipped timestamps and avoid panics.

Comment on lines 40 to 45
let info = unsafe {
INIT.call_once(|| {
INFO = Some(TimeZone::local().expect("unable to parse localtime info"));
});
INFO.as_ref().unwrap()
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially this caching behavior should be configurable. In a long running application, it might be unexpected that even when the system timezone changes, the application still uses the old timezone

@esheppa esheppa mentioned this pull request May 1, 2022
@esheppa esheppa changed the base branch from tz-info to main May 6, 2022 12:30
@esheppa
Copy link
Collaborator Author

esheppa commented May 12, 2022

Will try to recreate this to get CI running

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.

None yet

2 participants