-
Notifications
You must be signed in to change notification settings - Fork 170
Editorial: Move regulating and balancing logic into ISODateSurpasses #3138
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
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3138 +/- ##
==========================================
- Coverage 96.85% 96.77% -0.08%
==========================================
Files 21 21
Lines 9983 9992 +9
Branches 1829 1830 +1
==========================================
+ Hits 9669 9670 +1
- Misses 268 276 +8
Partials 46 46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Note: the new spec performs operations that may be redundant, like calling |
I'm going to prove that this is purely a reverse-inline refactoring. First, I'll take the new
Now I will inline the new steps of
Hopefully you agree so far: everything I've done is purely mechanical. Now I will make the following assertions:
With these simplifications, we get:
Now I will merge lines, rename some variables, and return to regular Repeat statements:
And voilà! We have returned to the current spec text. This concludes my proof that this PR is editorial. |
For the weeks and days part, we already used a shortcut instead of looping, so we keep it the same as before. For the years and months calculation, we change to the new definition of ISODateSurpasses.
b6df80c
to
52e277c
Compare
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 proof seems correct, thanks 😄
I have updated the reference code to use the new definition of ISODateSurpasses.
This is a big step toward making CalendarDateUntil be calendar-agnostic (#3136).
I have done my best to verify that the change is fully editorial. I wrote Rust code for the before and after and ran it for all date pairs between 2022 and 2026, and found no differences. You can find my code here:
https://gist.github.com/sffc/2c7811dab688a3d21c3e8a4478a8d1dc
The function named
CalendarDateUntil
is the current algorithm, ported line-by-line, andCalendarDateUntil4
is the new one. (Versions 2 and 3 are a sneak-peak of what I want to do in follow-up PRs to the spec)