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 step to eachDay function #487

Merged
merged 4 commits into from Jul 28, 2017
Merged

Add step to eachDay function #487

merged 4 commits into from Jul 28, 2017

Conversation

@BDav24
Copy link

@BDav24 BDav24 commented May 12, 2017

Hi, do you think this option would be useful?

Copy link
Member

@kossnocorp kossnocorp left a comment

@BDav24 I like the idea it's differently useful!

Now when v2 branch is merged your branch and code is out date 😩
Also, there are a couple of things that I think needs to be done before merge:

  • With v2 each function accepts options object: https://github.com/date-fns/date-fns/blob/master/docs/Options.js. It makes sense to add step to it. In the future, we may implement eachHour, eachSecond, etc. function so this option will be useful there as well 👍.

  • The option must check step for being a positive number otherwise it could lead to infinite loop, so we need to throw TypeError otherwise.

Loading

@BDav24
Copy link
Author

@BDav24 BDav24 commented Jul 14, 2017

Hi @kossnocorp, may I let you merge this feature then? Given the low complexity, you will go a lot faster than I would, setting up the whole env ;)

Loading

@kossnocorp kossnocorp changed the base branch from master to v1 Jul 28, 2017
@kossnocorp
Copy link
Member

@kossnocorp kossnocorp commented Jul 28, 2017

Okay! My plan is to merge and release it to v1 and then backport to v2. I'll ping once it's released.

Loading

@kossnocorp kossnocorp merged commit c38db25 into date-fns:v1 Jul 28, 2017
1 check passed
Loading
kossnocorp added a commit that referenced this issue Jul 28, 2017
kossnocorp added a commit that referenced this issue Dec 11, 2018
Backport the feature introduced in #487
kossnocorp added a commit that referenced this issue Dec 11, 2018
Backport the feature introduced in #487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants