NotificationsYou must be signed in to change notification settings
Fork5.7k
Star33.4k

fix: properly floor the partial results in`DateDayDifference`#1629

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

raklaptudirm merged 1 commit intoTheAlgorithms:masterfromvil02:fix_DateDayDifference

Mar 4, 2024

Merged

fix: properly floor the partial results in`DateDayDifference`#1629

raklaptudirm merged 1 commit intoTheAlgorithms:masterfromvil02:fix_DateDayDifference

Mar 4, 2024

Conversation

Copy link

Member

Describe your change:

CurrentlyDateDayDifference('29/02/2024', '04/03/2024') gives5. This PR fixes that. Further more:

the errors now arethrown,
the tests are reorganized; edited test cases were verified with the page linked in the docs,
some minor refactoring was done.
Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?

Checklist:

I have readCONTRIBUTING.md.
This pull request is all my own work -- I have not plagiarized.
I know that pull requests will not be merged if they fail the automated tests.
This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
All new JavaScript files are placed inside an existing directory.
All filenames should use the UpperCamelCase (PascalCase) style. There should be no spaces in filenames.
Example:UserProfile.js is allowed butuserprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not
All new algorithms have a URL in their comments that points to Wikipedia or another similar explanation.
If this pull request resolves one or more open issues then the commit message containsFixes: #{$ISSUE_NO}.

Copy link

codecov-commenter commentedMar 4, 2024•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.91%. Comparing base(83b4dd8) to head(3368c0d).

Additional details and impacted files

@@            Coverage Diff             @@##           master    #1629      +/-   ##==========================================+ Coverage   83.89%   83.91%   +0.01%==========================================  Files         377      377                Lines       19736    19733       -3       Branches     2916     2919       +3     ==========================================+ Hits        16558    16559       +1+ Misses       3178     3174       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

vil02 force-pushed thefix_DateDayDifference branch fromee9ca06 to4366be9Compare

March 4, 2024 17:00

fix: properly floor the partial results

3368c0d

vil02 force-pushed thefix_DateDayDifference branch from4366be9 to3368c0dCompare

March 4, 2024 17:03

vil02 marked this pull request as ready for review

March 4, 2024 17:06

vil02 requested review fromraklaptudirm andappgurueu ascode owners

March 4, 2024 17:06

appgurueu approved these changes

Mar 4, 2024

View reviewed changes

Copy link

Collaborator

appgurueu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks. BTW,CheckDayAndMonth should ideally compute how many days a month has (based on the year) and check against that (you don't need to do this though).

Copy link

MemberAuthor

vil02 commentedMar 4, 2024

CheckDayAndMonth should ideally compute how many days a month has (based on the year) and check against that (you don't need to do this though).

A function likeParseSimpleDate could be used both inDateToDay andDateDayDifference (i.e. the input would be a string"24/03/2024" and the output would be some kind of tuple with day, month and year). It should also check if the resulting date is correct (including correct number of days for given month). I was planning to spend some time on it after this and#1628 are merged.