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

[bugfix] Fix endOf for daylight saving time #4254

Closed
wants to merge 1 commit into from
Closed

[bugfix] Fix endOf for daylight saving time #4254

wants to merge 1 commit into from

Conversation

darakeon
Copy link

The endOf formula was not working for the day that the daylight saving
time starts.

Couldn't run tests locally, but running the test code worked.

resolves #4249

@jsf-clabot
Copy link

jsf-clabot commented Oct 18, 2017

CLA assistant check
All committers have signed the CLA.

test('endOf DST day', function (assert) {
// Based on a real story somewhere in Brazil/Sao_Paulo
var dstChangeDay = moment('2017-10-15T12:00:00-03:00');
var dstChangeDayAfter = moment('2017-10-16T12:00:00-03:00');
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming you mean America/Sao_Paulo, the offset would be -02:00 the day after.

The problem here is that this test will be running on whatever the local time zone happens to be - not necessarily in Brazil.

I think though the code change needs to happen in moment, we would need to bring moment-timezone into the unit tests (or move the tests into moment-timezone) in order to start testing DST changes.

Copy link
Author

Choose a reason for hiding this comment

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

I copied another test in same file to make my, both should be moved?

Copy link
Author

@darakeon darakeon Nov 17, 2017

Choose a reason for hiding this comment

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

Should I remove the test and left just the code?

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

@mj1856 , removed tests. I'll try to make them on https://github.com/moment/moment-timezone

@darakeon darakeon changed the title Fix endOf for daylight saving time [bugfix] Fix endOf for daylight saving time Jan 18, 2018
At timezone GMT-3, in Brazil, the last day before timezone change had
one more hour than it should have.
@darakeon
Copy link
Author

Seems like it's not needed anymore.

@darakeon darakeon closed this Feb 14, 2018
@darakeon darakeon deleted the fix/daylight-saving-time branch February 14, 2018 21:52
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.

Daylight saving time issue
4 participants