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

addWeekday(s) clears time #54

Closed
dereuromark opened this issue Nov 12, 2015 · 4 comments · Fixed by #58
Closed

addWeekday(s) clears time #54

dereuromark opened this issue Nov 12, 2015 · 4 comments · Fixed by #58
Labels
bug
Milestone
0.5.0

Comments

@dereuromark
Copy link
Member

Check if
briannesbitt/Carbon@d02c413
needs to be applied

Refs briannesbitt/Carbon#419
@lorenzo
Copy link
Member

lorenzo commented Nov 19, 2015

@dereuromark would you like to copy the fix from them?

@markstory markstory added this to the 0.5.0 milestone Nov 19, 2015
@dereuromark
Copy link
Member Author

It seems that their fix only was for addWeekday. addWeekdays as well as the sub methods also need fixing i guess.
Additionally, the fixes cannot be applied without some modification for us: https://github.com/cakephp/chronos/compare/master-weekday
Tests failing ( https://travis-ci.org/cakephp/chronos/builds/92567642 ).

@markstory
Copy link
Member

@dereuromark Those changes seem reasonable. For the immutable objects, you'll need to capture the return of modify() and chain off of that instead.

@dereuromark
Copy link
Member Author

You are absolutely right. Done :)

@dereuromark dereuromark mentioned this issue Nov 22, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
0.5.0
Development

Successfully merging a pull request may close this issue.

Fix: addWeekday(s) clears time cakephp/chronos
3 participants
@markstory @lorenzo @dereuromark