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

Fix Duration::MIN.abs() (adjust Duration::MIN by 1 millisecond) #1334

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

pitdicker
Copy link
Collaborator

@pitdicker pitdicker commented Sep 29, 2023

Currently Duration::MIN.abs() will create a value that is 1 millisecond beyond Duration::MAX.abs().
We can choose to add a fallible variant of Duration::abs.
But given that Duration::MIN is pretty much chosen arbitrarily as i64::MIN milliseconds, I propose to redefine it as -i64::MAX milliseconds and make the range symmetrical.

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #1334 (70052e0) into 0.4.x (fe4713a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##            0.4.x    #1334   +/-   ##
=======================================
  Coverage   91.55%   91.55%           
=======================================
  Files          38       38           
  Lines       17352    17355    +3     
=======================================
+ Hits        15886    15889    +3     
  Misses       1466     1466           
Files Coverage Δ
src/duration.rs 92.93% <100.00%> (+0.04%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pitdicker
Copy link
Collaborator Author

Wow, approved before the CI is even ready!

@pitdicker pitdicker merged commit 0f19d6b into chronotope:0.4.x Sep 29, 2023
37 checks passed
@pitdicker pitdicker deleted the duration_abs_min branch September 29, 2023 11:42
danwilliams added a commit to danwilliams/rubedo that referenced this pull request Jan 28, 2024
  - Adjusted MIN_MILLISECONDS, MIN_NANOSECONDS_FULL, and
    MIN_MICROSECONDS_FULL due to changes in Chrono 0.4.32 that were not
    previously noticed, which restrict the minimum value of milliseconds
    by 1, by changing the limit from i64::MIN to -i64::MAX. This was
    carried out under chronotope/chrono#1334.
    Notably, Chrono 0.4.32 does still allow creation of a Duration with
    a milliseconds value of i64::MIN, which is why this change was not
    noticed sooner. This will be corrected when Chrono 0.4.34 is
    released, with chronotope/chrono#1385. In
    the meantime, the correction of constant values here means that we
    are now compatible with both versions and also do not suffer from
    the current Chrono bug.
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