search

azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.05k stars 155 forks source link

Separated czech and slovak independence day from New Year's Day #158

Closed janlanger closed 5 years ago

janlanger commented 5 years ago

As I understood the code, the translations in data/translations should be perfectly equivalent. In Czech republic and Slovakia there are two holidays celebrated on 1st of January - New Year's Day and Disolution of Czechoslovakia.

So I think Czech and Slovak holiday providers should not use newYears method, but have its own definition. Current code causes czech version "Day of renewal of the independent Czech state and New Year's Day" to be translated to "Day of the Establishment of the Slovak Republic" in slovak.

stelgenhof commented 5 years ago

Indeed, the translation of a holiday is a 1:1 match for that holiday. You can create multiple holidays that fall on the same day. Using the addHoliday method you can add as many as you like as long as the internal name (shortName) is unique. Then for each of those holidays you add the corresponding translation.

Your PR works too, however if you prefer to see New Year's Day and Disolution of Czechoslovakia as two separate dates generated by Yasumi, I recommend to create two holidays as mentioned earlier.

Cheers! Sacha

janlanger commented 5 years ago

Well, I would prefer having only one holiday instance for one date. Can this PR get merged?

stelgenhof commented 5 years ago

After doing a bit more research and re-reading your initial notes, my understanding is that the situation is the following:

Slovakia: Celebrates the "Day of the Establishment of the Slovak Republic" on January 1st (since 1993) Czechia: Celebrates the "Day of renewal of the independent Czech state" (since 1993) and "New Year's Day".

Is that correct? Just want to see how the holidays are defined, before talking about code changes/PR :)

Cheers! Sacha

janlanger commented 5 years ago

Yes, thats correct.

stelgenhof commented 5 years ago

Thanks! So the case for Slovakia is easy and for Czechia a bit less easy.

Slovakia: Since they don't celebrate the common 'New Years Day', New Years Day can be replaced by Slovakia's own holiday representing "Day of the Establishment of the Slovak Republic". This needs to be with its own internal id and the translation can be done within the provider class.

The Slovakian translation of "New Years Day" can be left as is within the New Years Day translation file, but it may be better to remove it to avoid any confusion.

Czechia: Czechia celebrates two events on January 1st: the "Day of renewal of the independent Czech state" (since 1993) and "New Year's Day". Although this could be combined as one holiday, I recommend to create two separate holiday calculations. For "New Years Day", you can leave everything as is. For "Day of renewal of the independent Czech state", a new holiday needs to be created with its own id (The same way as described above for Slovakia).

For both Slovakia and Czechia, "Day of the Establishment of the Slovak Republic" and "Day of renewal of the independent Czech state" resp. are established at the time of the separation of the two countries (1993). This needs also be reflected in the Holiday provider classes and unit tests.

Cheers! Sacha

stelgenhof commented 5 years ago

@janlanger Could you have a look at my last comment? If you agree with my summary, could you make adjustments to your PR then? I'd like to include your PR in the next v2.2 release.

Thanks! Sacha

janlanger commented 5 years ago

@stelgenhof Hi, yes, your summary is correct. I'll try to do it this week.

janlanger commented 5 years ago

@stelgenhof updated