-
Notifications
You must be signed in to change notification settings - Fork 35
migrate lazy_static -> std::sync::LazyLock #1011
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
Conversation
LazyLock was stabilized in 1.80, we can drop our dependency on lazy_static. This frees us up from having to keep the dependency up to date with libraries that are still using it.
| /// again when an inactive hotspot's h3 index would otherwise be garbage-collected | ||
| /// from density scaling calculations and not finding a value on subsequent lookups | ||
| /// would disqualify the hotspot from validating further beacons | ||
| static DEFAULT_TX_SCALE: LazyLock<Decimal> = LazyLock::new(|| Decimal::new(2000, 4)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this decimal wrapped in a LazyLock, but in the next file there’s a half-dozen or so static decimals declared using the dec! macro without any LazyLock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing I was attempting to change in this PR is the use of lazy_static. All the constructors were left as untouched as possible.
We could ask a similar question about the difference between
dec!(0.2), dec!(0.20), dec!(0.200), and dec!(0.2000).
Decimal::new(2000, 4), Decimal::new(200, 3), Decimal::new(20, 2), and Decimal::new(2, 1).
They're most likely all the same. I don't believe this PR is the place to find that out for a static that's been in use for 2 years and has no reason to change.
| *BEACON_INTERVAL + Duration::from_secs(2 * 60 * 60); | ||
| } | ||
| static BEACON_INTERVAL: Duration = Duration::from_secs(21600); | ||
| static BEACON_INTERVAL_PLUS_TWO_HOURS: LazyLock<Duration> = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you can create and add std::time::Duration s in a const context, does this need the LazyLock wrapper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are std::time::Duration. You cannot add them in a const context.
If you remove the LazyLock, the compiler will lead you to this tracking issue.
LazyLockwas stabilized in1.80, we can drop our dependency onlazy_static. This frees us up from having to keep the dependency up to date with libraries that are still using it.This PR is preparing for further updates to aws dependencies