handle invalid timezones Australia/ACT and Europe/Kiev #343
Conversation
…oduces undesired changes Signed-off-by: Sacha Telgenhof <[email protected]>
* Update methods visibility in multiple Providers Methods in several providers including Switzerland, South Korea, and Luxembourg among others have been updated from private to protected. These changes allow for better extensibility and customization of the Yasumi library. The methods affected are those calculating specific holidays in the different regions.
Signed-off-by: Sacha Telgenhof <[email protected]>
Signed-off-by: Sacha Telgenhof <[email protected]>
Using the latest version of the PHP CS configuration, update all source code to use the most recent code style and formatting rules. For the most part this updates the header comment and the strict type declaration. It should not have any affect on the working of the tests nor the main code. Signed-off-by: Sacha Telgenhof <[email protected]>
The generator for the years on or after the establishment of the Portuguese Republic Day did not honor the fact that this holiday was suspended in 2013, resulting in tests to fail for years between 2013 and 2016 (when it was restored again). Signed-off-by: Sacha Telgenhof <[email protected]>
One-time holiday was added on 30.11.2018 and removed 1.9. since 2024, because our Slovak government has officially decided that we don't need so many days to rest 🤦🏼
Implement provider for Iran
…in Brandenburg (azuyalabs#337) * fix(Provider\Germany): pentecost is not an official holiday - except in Brandenburg (see azuyalabs#100)
Signed-off-by: Sacha Telgenhof <[email protected]>
Signed-off-by: Sacha Telgenhof <[email protected]>
|
Thank you very much @kevinpapst for this PR! Unfortunately there are sometimes backwards incompatible changes introduced in PHP. In the past there were similar BC issues with historical DST changes between PHP versions. We could also move away from the timezone identifiers (type 3) altogether, and use the UTC offset (type 1) or the timezone abbreviation (type 2), as these seem less prone to changing. The benefit of the timezone identifier is of course it is more human friendlier than the other types. |
|
Yeah, I fought in the past already with the I think the timezone identifier is totally fine to use, it doesn't change regularly and the try&catch approach works flawless to handle this situation. Changing it to TZ offset would be a bigger BC break, as many of the How to move forward?
|
|
Yeah, wouldn't think the timezone identifier changes regularly either, but thought the others could be an option instead. However indeed that would break Yasumi's API :( I think the try&catch approach is a good approach. Please go ahead and use the same for Australia. Let me check on that dead catch issue in regards to Psalm. |
|
I already did change Australia. |
|
Ok for the Changelog, but tell me how to test these changes. The result depends on the environment / PHP runtime. |
|
Hi @stelgenhof @kevinpapst ! I just wanted to suggest using phpversion() . It's self explaining and better than using a try catch |
|
Does not depend on phpversion afaik, so this approach is not usable. |
|
I was thinking the same to use the PHP version for testing. If that turns out to be difficult, we can forgo the tests :) |
|
@kevinpapst Can you only update the changelog? If done, then I'll merge it. Thanks! |
|
@stelgenhof Changelog adjusted: done. Any chance you could create a bugfix release? I have production bugs with several users. |
| @@ -14,12 +14,15 @@ changes. | |||
|
|
|||
| ### Changed | |||
|
|
|||
| - Holiday calculation methods in providers are now protected instead of private | |||
| - Holiday calculation methods in providers are now protected instead of private | |||
There was a problem hiding this comment.
sorry, that must have been my IDE 🤷
|
Sorry about the late reply. Somehow I missed any of the GitHub notifications. |
stelgenhof
force-pushed
the
develop
branch
8 times, most recently
from
4ef0ec7 to
6912aaa
Compare
|
Hello @stelgenhof , I keep on having users who run into this bug. I understand if you don't want to merge this: its open source, your product and there is no warranty given. But I am unfortunately getting bug/support requests every other week and I have no proper answer since a few months. I don't want to cause you any inconvenience here, please don't get that wrong: I appreciate your work and wanna thank you for sharing Yasumi! But I have to provide a bugfix after such a long time and I can't wait any longer. I don't want to maintain a fork, so I wanted to reach out one more time to ask, if you could share your plans. Is there any chance this gets incorporated anytime soon in a new release? |
|
Just today: |
|
Hi @kevinpapst Let me have a look this weekend to make a release. |
stelgenhof
force-pushed
the
develop
branch
3 times, most recently
from
f3c55da to
054359c
Compare
Fixes #342
I hope the code comments explain the reasoning behind the changes.
All my systems seem to support both variants, but I had multiple bugs raised about
Australia/ACTandEurope/Kiev.Can't say when this happens, maybe it depends on the
tzdatalib on the system where PHP was compiled. Or some PHP builds remove all timezone name which are only so called "links".In the official table (see https://en.wikipedia.org/wiki/List_of_tz_database_time_zones ) both problematic TZ are only links, so I replaced them with their real TZ identifier.
Edit: I used two approaches, so you can decide which way is the better one.
Use the correct TZ identifier directly (Sydney) or use the try&catch approach (Kiev).
Maybe try&catch in both places is the best approach for maximum BC?