Skip to content

Conversation

ComaVN
Copy link

@ComaVN ComaVN commented Nov 10, 2021

This is a fix for bug #68549

It's not as clean as I hoped, but it's extensively unit tested

The fix would probably be simpler if the timelib_time_offset type also contained the timestamp of the next DST transition, but that seemed like too big of a change to me.

@ComaVN ComaVN changed the title Issue 68549 Issue 68549: Timezones and offsets are not properly used when working with dates Nov 10, 2021
@cmb69
Copy link
Member

cmb69 commented Nov 10, 2021

Please submit PRs for ext/date/lib/* to https://github.com/derickr/timelib.

@ComaVN
Copy link
Author

ComaVN commented Nov 11, 2021

Please submit PRs for ext/date/lib/* to https://github.com/derickr/timelib.

I'd do that, but I'm having trouble figuring out how... the last commit on PHP-7.4 seems to indicate it's version 2021.5, but that version's code doesn't look to be the same as what's in ext/date/lib...

@cmb69
Copy link
Member

cmb69 commented Nov 11, 2021

timelib is on 2021.10 right now. Still, try to fix it there, and then a back-port of the fix might be viable.

@ComaVN
Copy link
Author

ComaVN commented Nov 12, 2021

timelib is on 2021.10 right now. Still, try to fix it there, and then a back-port of the fix might be viable.

According to CONTRIBUTING.md, bugfixes are against the lowest branch first, right? and that's definitely not on 2021.10...

I'll see what I can do in https://github.com/derickr/timelib

…ing with dates)

Particularly when setting timestamps or changing timezones during DST changes
@ComaVN
Copy link
Author

ComaVN commented Nov 14, 2021

I managed to fix it a lot easier, without changing timelib. I still think there's a bug there, it's just not noticable here anymore.

@derickr
Copy link
Member

derickr commented Nov 14, 2021

This doesn't look right. The function should be able to be called regardless of whether the sse is up to date

@ComaVN
Copy link
Author

ComaVN commented Nov 14, 2021

The function should be able to be called regardless of whether the sse is up to date

Well, it clearly cannot. I'm currently busy trying to get an MR working for timelib tho.

@cmb69 cmb69 added the Bug label Nov 29, 2021
@cmb69
Copy link
Member

cmb69 commented Nov 29, 2021

I'm currently busy trying to get an MR working for timelib tho.

That is derickr/timelib#121.

@derickr
Copy link
Member

derickr commented May 13, 2022

This PR can be closed, as the relevant code has made it into timelib now and will automatically filter down here when it gets merged.

@derickr derickr closed this May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants