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

Year setter should keep time when DST changes #3036

Closed
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
3 participants
@mj1856
Member

mj1856 commented Mar 13, 2016

Consider:

moment.tz("2016-03-28T00:00:00","Europe/London").year(2015).format()  //  "2015-03-27T23:00:00+00:00"

It should have been "2015-03-28T00:00:00+00:00"

While this only manifests when using moment-timezone, the problem is in moment itself. The keepTime flag of the year unit should be set to true, as it is for months, days, and hours.

Fixed this, added some unit tests, and added similar tests around other units and related scenarios.

@maggiepint

This comment has been minimized.

Member

maggiepint commented Mar 20, 2016

Code looks fine, but did you want a commit with no tests?
I have a weird thing about always wanting to see test coverage with every commit when reading history, but maybe that's just me.

@mj1856 mj1856 added this to the 2.13.0 milestone Mar 23, 2016

@mj1856

This comment has been minimized.

Member

mj1856 commented Mar 23, 2016

Re-based to one commit so tests are associated in the commit history. Thanks.

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 12, 2016

@mj1856 hm, weird why it was created broken. Thanks for the fix!

@ichernev

This comment has been minimized.

Contributor

ichernev commented Apr 16, 2016

Merged in e52fd3a

@ichernev ichernev closed this Apr 16, 2016

ichernev added a commit that referenced this pull request Apr 16, 2016

Merge pull request #3036 from mj1856:set-across-dst
Year setter should keep time when DST changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment