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

Adding Swiss provider #56

Merged
merged 4 commits into from
Feb 28, 2017
Merged

Adding Swiss provider #56

merged 4 commits into from
Feb 28, 2017

Conversation

qligier
Copy link
Contributor

@qligier qligier commented Feb 23, 2017

This PR adds common holidays of Switzerland (including the sub-regions Aargau, Appenzell Ausserrhoden, Appenzell Innerrhoden, Basel Landschaft, Basel Stadt, Bern, Fribourg, Geneva, Glarus, Grisons, Jura, Lucerne, Neuchatel, Nidwalden, Obwalden, Schaffhausen, Schwyz, Solothurn, St. Gallen, Thurgau, Ticino, Uri, Valais, Vaud, Zug and Zurich).

Except for the Swiss national day, holidays are to be chosen by cantons, not by the government so a provider for each canton is provided.

The PR includes the providers, unit tests and translation of common holidays.

@stelgenhof
Copy link
Member

@qligier Thank you so much for this! Must have been quite a job. Let me have a look and merge your PR if all looks fine 👍

@stelgenhof stelgenhof self-assigned this Feb 24, 2017
@stelgenhof stelgenhof added this to the v1.7.0 milestone Feb 24, 2017
@stelgenhof stelgenhof self-requested a review February 24, 2017 01:05
@qligier
Copy link
Contributor Author

qligier commented Feb 24, 2017

It has been quite hard because of the fragmentation of holidays visible in this doc (in blue, holidays that only exist in sub-sub-regions).

I've implemented all holidays except the Swiss national day as other. They could also be set as national or we could create a regional type.

@stelgenhof
Copy link
Member

After briefly checking your PR, I realize that the type 'NATIONAL' is not quite appropriate. My intention was this one to reflect the official holidays irrespective of the region/subregion. I think I am going to change the naming for that so it better reflects this situation.

@qligier
Copy link
Contributor Author

qligier commented Feb 24, 2017

Renaming national to official could be a good thing (even if bank and seasonal holidays also are official). Differentiate between national and regional holidays is maybe not that important. That could also be useful for Australia, France, Germany and Spain.

@stelgenhof
Copy link
Member

Yes, the main purpose of the different types was to differentiate between official/government approved holidays and other (observed, seasonal, others, etc.) type of holidays. Perhaps that needs a refactoring :)

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.

As discussed, the holiday type to be used for subregions might need some rethinking in Yasumi, so using 'OTHER' is fine for now. We will see if other people experience issues with that :). All is looking good. Thanks for the hard work!

@stelgenhof stelgenhof merged commit fa83a4a into azuyalabs:master Feb 28, 2017
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