-
-
Notifications
You must be signed in to change notification settings - Fork 197
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
Add French holidays #15
Conversation
Hi @levrailoup
Also, in the departments of Moselle, Bas-Rhin and Haut-Rhin, the list is slightly different: https://www.legifrance.gouv.fr/codes/article_lc/LEGIARTI000006902635 In Guadeloupe, Guyane, Martinique, Mayotte, La Réunion, Saint-Barthélemy and Saint-Martin, which are all part of France, there are additional holidays: https://www.legifrance.gouv.fr/codes/article_lc/LEGIARTI000035902463 I am not sure if this package handles regional differences. |
Lundi de Pentecôte, Ascension and Lundi de Pâques are already included in the PR : As they are dependent upon Easter date, they can be found in France::variableHolidays(). As for the other dates you mention... I'm not quite sure what is expected from this package : should we have a specific class for each region ? Country codes don't include regions AFAIK, so it might prove difficult. "Vendredi Saint" is even town-specific... In the meantime, the current PR includes all common holidays for mainland France. |
Co-authored-by: Hugo Alliaume <[email protected]>
src/Countries/France.php
Outdated
return 'fr'; | ||
} | ||
|
||
/** @return array<string, CarbonImmutable> */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this return type is wrong (but anyway, you should not need to specify it here. Inheriting it from the parent is better)
I've added documentation on how I think we should manage regions or states: |
My bad. I must have forgotten to open my eyes 😅 |
Could you rebase this branch, I think it should allow the tests to run here |
This PR looks good! I'll wait for the department based holidays before merging this. I've added documentation on how I think we should manage regions, states or departments: |
Added support for region-specific holidays. It uses ISO 3166-2 codes for now, but I am open to suggestions. |
Thanks for your work on this one! |
No description provided.