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

Extract the timezone API from time 0.1 #459

Closed
6 tasks
Plecra opened this issue Jul 22, 2020 · 2 comments
Closed
6 tasks

Extract the timezone API from time 0.1 #459

Plecra opened this issue Jul 22, 2020 · 2 comments

Comments

@Plecra
Copy link

Plecra commented Jul 22, 2020

This is part of addressing #400.

I think it would be useful to have a small utility crate akin to getrandom to fetch a timestamp's local UTC offset.

This would effectively implement chrono::offset::Local (on second thought, this probably isn't true. It might not be enough for TimeZone::offset_from_utc_datetime. Gonna need to think about that), but would also be very useful for simpler time-related code that doesn't want to depend on chrono. Since the plan seems to be for chrono to absorb the functionality it uses from time, it seems to make sense for this timezone crate to be maintained under chrono.

I don't think the API needs to be particularly complex: we can draw from getrandom here.

mod the_crate {
    /// Errors that can occur during resolution. This will depend on the implementation.
    // Do we care about specific OS errors that the implementation encounters? I can't
    // think of any use for them
    pub struct Error {
        DoesntExist,
        Ambiguous(?), // Are there any cases of three possibilities?
        NoLocalTimezone,
    }
    /// Get the UTC offset of the system at a UNIX `timestamp`.
    ///
    /// This includes any daylight savings offset applied by the system.
    pub fn local_utc_offset_at(timestamp: i64) -> Result<Duration, Error>;
}

Questions

  • Is the chrono team okay with taking this on?
  • Is a UNIX timestamp the most appropriate input to use? Should we use a platform-dependent representation with something like SystemTime::UNIX_EPOCH?
    • This does raise the question of whether an isolated local timezone API makes sense. Maybe it belongs with some other tightly coupled functions
  • Do we know enough about timezones to prepare a 1.0 release?
    • Maybe this could take a core::time::SystemTime if it moves from std? Would we want that?
  • Can we write an implementation that doesn't use a system API for standalone context? Can that make sense?

To be clear, I'm happy to implement this myself, I would just like to hear opinions on the implementation before tackling it

@pitdicker
Copy link
Collaborator

For context: this issue is from before chrono absorbed the platform parts and Duration type from time 0.1 in #478.

It seems like we are moving in a different direction then this issue supposes. Instead of a thin wrapper around system functions, we try to get the time zone data from the OS and do the calculations within chrono.

@djc
Copy link
Member

djc commented Sep 2, 2023

I think this is too far from the current design and we don't have resources to go do a bunch of extra stuff.

@djc djc closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2023
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

No branches or pull requests

3 participants