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 time with chrono #23

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Replace time with chrono #23

merged 1 commit into from
Jul 18, 2023

Conversation

mibmo
Copy link

@mibmo mibmo commented Jul 4, 2023

Removes the old time dependency and switches all use to the more modern chrono crate. Tests pass but would love a proper review from someone familiar with the codebase. :)

I'm a bit concerned about the previous use of time::empty_tm(), as the documentation is a bit lacking, but I'm assuming that since it's mostly used for setting issuers' touched, it's used to represent the earliest possible time, so I've replaced it with chrono::DateTime::MIN_UTC.

Closes #9

@srhb srhb self-requested a review July 18, 2023 06:04
@srhb
Copy link
Contributor

srhb commented Jul 18, 2023

Looks great, thank you! Your analysis about empty_tm seem correct to me (it's a more complicated version of "zero seconds from unix epoch", ie. zero days from epoch, zero weeks from epoch, zero ... from epoch) so your translation appears absolutely correct.

@srhb srhb merged commit e3146ac into DBCDK:main Jul 18, 2023
1 check passed
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.

Replace "time" with "chrono"
2 participants