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

International Womens Day #133

Merged
merged 5 commits into from
Feb 12, 2019
Merged

Conversation

huehnerhose
Copy link
Contributor

International Womens Day is a one time holiday in Germany/Berlin in 2019.

Since it is a common holiday in Russia, I moved it to common holidays and added german localisation and the Germany/Berlin exception for 2019.

I moved it to common holidays, because I think women are at least as important as animals. (World Animal Day seems only be used in the Netherlands but is in commonHolidays, I think International Womens Day deserves its place there, too)

add german localisation;
add international womens day as single time berlin holidy;
@stelgenhof
Copy link
Member

Thanks for the PR! Any holiday can be added, as long as they are official/national or a defacto holiday in the respective provider (country) :) Just make sure you assign the correct type.

Cheers! Sacha

Copy link
Member

@stelgenhof stelgenhof left a comment

Choose a reason for hiding this comment

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

Since you've placed this in the common holiday provider class, a general unit test would be needed to.
There is only a unit test for Berlin, so also tests covering other countries as Russia are not covered.

src/Yasumi/Provider/CommonHolidays.php Show resolved Hide resolved
add Ukraine (uk_UA, ru_UA) locales for internationalWomensDay
@huehnerhose
Copy link
Contributor Author

huehnerhose commented Feb 8, 2019

I updated Ukraine, since it also used a custom holiday add.

Regarding the general unit test: I don't see where I should add this? There are tests for Russia, Ukraine and Germany/Berlin, which work now. Please give me a hint where I should add the general test.

Thanks!

Edit: I realised: International Women's Day becomes a holiday 2019 in Berlin. It isn't a one time thing.

@stelgenhof
Copy link
Member

@huehnerhose Thanks! Ignore my comment regarding the general unit test: I thought I always did that for common holidays, but that is not the case. Your unit tests are fine :)

Only one last request: Can you add an entry to the CHANGELOG.md file describing this change? That would be great. Once done, I'll have a final check and will merge it.

Cheers! Sacha

@huehnerhose
Copy link
Contributor Author

Sure :) and done.

@stelgenhof stelgenhof merged commit f756448 into azuyalabs:master Feb 12, 2019
@stelgenhof stelgenhof added this to the v2.1.0 milestone Mar 28, 2019
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.

2 participants