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

add new function roundToNearestMinutes #928

Merged
merged 2 commits into from
Oct 18, 2018
Merged

Conversation

xkizer
Copy link
Contributor

@xkizer xkizer commented Oct 13, 2018

Fixes #787

Copy link
Member

@kossnocorp kossnocorp left a comment

Choose a reason for hiding this comment

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

Neat! Thank you a lot!

@kossnocorp kossnocorp merged commit 9b0cfbf into date-fns:master Oct 18, 2018
@kossnocorp
Copy link
Member

The PR was released as v2.0.0-alpha.23. See the change log: https://gist.github.com/kossnocorp/a307a464760b405bb78ef5020a4ab136#v200-alpha23

Thank you for your great work!

@dcrystalj
Copy link

this is very limited just 1 to 30 minutes. What about 33 minutes ?
Also there should be option to round to nearest seconds, hours. days, weeks...

@kossnocorp
Copy link
Member

@dcrystalj please elaborate your use case for rounding to 33 minutes. Regarding the seconds, hours, whatever, the function is called roundToNearestMinutes so it's should not.

@dcrystalj
Copy link

@kossnocorp sorry my point was just that we need more generic function. I would expect of this fn to work if I give it roundToNearestMinutes(date, 120) same as roundToNearestHour(date, 2)

RIght now in my project I need function to round to nearest hour, 3 hours, day and 3 day... Maybe we should just open issues to implement roundToNearestHour, and roundToNearestDay ...

@kossnocorp
Copy link
Member

@dcrystalj yeah, that makes sense. Feel free to open those, but also send PRs.

@xkizer
Copy link
Contributor Author

xkizer commented Oct 29, 2018

@kossnocorp if you file a separate issue I can try to work on it. Thanks for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants