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

Replace chrono with std::time #9

Closed
wants to merge 2 commits into from
Closed

Conversation

ebarnard
Copy link

I made this change for a personal project but though I'd make a PR in case it is wanted.

Chrono provides many advanced timezone and date handling features but none of these are used by timer.rs and it pulls in a large number of dependencies.

For users currently using chrono there are From and Into implementations to convert between chrono::DateTime and std::time::SystemTime.

@ebarnard
Copy link
Author

CC #7

@Yoric
Copy link
Owner

Yoric commented Sep 11, 2017

@ebarnard How does this impact the (admittedly few) crates that depend on timer.rs?

@ebarnard
Copy link
Author

This would require a minor version bump so those crates would have to upgrade manually to this version.
Users of the chrono Duration based apis would have to change func(duration) to func(duration.to_std()).
Users of the chrono DateTime based apis would have to change func(time) to func(time.into()).

@WiSaGaN
Copy link

WiSaGaN commented Feb 28, 2018

+1 for this. Depending on std types improves interoperability. You can also take advantage of the upcoming const fn Duration constructor.

@0xpr03
Copy link
Contributor

0xpr03 commented Feb 28, 2018

I'll add my two cents as I'm cc'ed:
I'm not in favor of changing it currently as chrono AFAIK still has the best API for parsing, computing & handling very different types of time representations.
What I saw recently was a crate that could handle both, chrono & std types. Sadly I don't remember the name anymore.
I was against chrono a while back and using a unified type is still far better, library wise. Also because chrono (a little bit like Rust's borrow checker) doesn't allow you to do things that easily which could break your time representation, making it at first a little bit less intuitive. But you can't represent every time aspect with the std type (or for example parse config times that easily) as you can with chrono. Call me lazy but I'm in favor of "never change a running system" here or support both types, which obviously would be the best solution.

(Which reminds me of the daylight savings related problem when starting a job at a specific hour and repeating it every 24 hours. But that is another issue.)

@ebarnard ebarnard closed this Sep 3, 2019
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.

4 participants