-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
roundToNearestMinutes
fixes and improvements
#3132
roundToNearestMinutes
fixes and improvements
#3132
Conversation
@im28 Would you mind reviewing the PR, especially the unit tests to make sure everything checks out and there are no missing edge cases? |
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.
Looks good to me, Thanks a lot!
Just pushed an update to cover an edge case with |
Hi, would someone be able to fix these merge conflicts so this can be released? I'm relying on this fix for a development! Thanks :) |
It seems like @leshakoss @tan75 I'm going to wait for you to chime in before changing anything. If you want to accept the PR, let me know if you want to merge into the Obviously the fixes in this PR should find their way to both v2 and upcoming v3. |
Any updates on this PR? |
While this PR hasn't been merged, I'm using this simple custom solution https://stackoverflow.com/a/10789415 |
Would also appreciate this being merged. |
The problem is that gh CLI conflicts with it
337155f
to
92e7b3e
Compare
Closes #3129
The recent addition of the
roundingMethod
option (#3091) was not implemented correctly, and some unit tests were not checking for the right behavior. This PR fixes the problems, adds several unit tests, improves the JS docs and adds two additional examples.I've also internally renamed the
getRoundingMethod
file and made the function as a default export for consistency to match other functions in the library.