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.

Sign up for GitHub

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

Jump to bottom

fix for recurring full day events east of UTC #42

Merged
merged 4 commits into from
Oct 27, 2020
Merged

fix for recurring full day events east of UTC #42

merged 4 commits into from
Oct 27, 2020

Conversation

sdetweil
Copy link
Contributor

fix #36

@sdetweil
Copy link
Contributor Author

sdetweil commented Oct 27, 2020 •
edited
Loading

lint forced change to includes from indexOf.. I didn't check the results of the function.. 'assumed' it was the same... not..

@jens-maus
Copy link
Owner

Thanks for the new PR. this looks much cleaner. The only thing missing is, that you please should add a new dedicated test for your particular change/enhancement so that in future npm test will test your fix and no further regression happens in case another change might bring back an issue again. So please add a new testcase to this PR.

jens-maus
jens-maus reviewed Oct 27, 2020
View reviewed changes
test/test-async.js Outdated Show resolved Hide resolved
test/test-async.js Show resolved Hide resolved
@jens-maus
Copy link
Owner

perfect! thanks for the nice contribution!

@jens-maus jens-maus merged commit cbb76af into jens-maus:master Oct 27, 2020
@sdetweil sdetweil deleted the fix-recurring-wholeday-events branch October 27, 2020 15:15
@sdetweil
Copy link
Contributor Author

should be 12.3, right? as u already published 12.2

@jens-maus
Copy link
Owner

yes, as soon as I will publish 0.12.3 this change will be included. I am just hoping someone can take care of #14 so that I can release it ASAP.

@sdetweil
Copy link
Contributor Author

got it.. we have a matching issue, but haven't been able to get anyone to send a testcase that actually fails.
I would fix it, as soon as I see a testcase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jens-maus jens-maus jens-maus left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

full day repeating rule event east of utc get initial date wrong.
2 participants
@sdetweil @jens-maus