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

Bug 1822482 - Switch from chrono to time 0.3 #2429

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

badboy
Copy link
Member

@badboy badboy commented Mar 23, 2023

This is a first try at migrating from chrono to time. This compiles, but with warnings.
Tests don't compile, as they still largely make use of chrono.

Things that would need to be tested:

  • Do we handle timezones correctly?
  • Are we handling time measurement correctly?
    • This switched from time::precise_time_ns() to libstd's Instant. We need to verify the guarantees.
  • Are we correctly (de)serializing data?
    • Datetimes are stored in the database. We need to handle the old data transparently. DatetimeWrapper should do that, but will require more testing.
  • Truncation/Formatting
    • We use TimeUnit for the date/time related metrics to truncate. As this switches formatting we need to verify it's correct (especially at the nano-/micro-/millisecond level)

This is a frist try at migrating from chrono to time.
This compiles, but with warnings.
Tests don't compile, as they still largely make use of chrono.

Things that would need to be tested:

* Do we handle timezones correctly?
* Are we handling time measurement correctly?
  * This switched from `time::precise_time_ns()` to libstd's `Instant`.
    We need to verify the guarantees.
* Are we correctly (de)serializing data?
  * Datetimes are stored in the database. We need to handle the old data
    transparently. `DatetimeWrapper` should do that, but will require
    more testing.
* Truncation/Formatting
  * We use `TimeUnit` for the date/time related metrics to truncate.
    As this switches formatting we need to verify it's correct
    (especially at the nano-/micro-/millisecond level)
@badboy badboy requested a review from a team as a code owner March 23, 2023 14:06
@badboy badboy requested review from rosahbruno and removed request for a team March 23, 2023 14:06
@glandium
Copy link
Contributor

This switched from time::precise_time_ns() to libstd's Instant. We need to verify the guarantees.

FWIW, I looked into this last week or the week before, and libstd has the same, or better, precision.

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.

2 participants