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

Add French subdivision aliases #2162

Merged
merged 1 commit into from
Dec 12, 2024
Merged

Conversation

bors-ltd
Copy link
Contributor

Proposed change

I had to look up in the source code what MQ, RE... meant. The README didn't help either, just listing the raw codes available.

Those are not of common use in the French culture, contrary to say, AZ meaning Arizona in the US culture.

Also fixed the official spelling.

Type of change

  • New country/market holidays support (thank you!)
  • Supported country/market holidays update (calendar discrepancy fix, localization)
  • Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
  • Dependency update (version deprecation/pin/upgrade)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (a code change causing existing functionality to break)
  • New feature (new holidays functionality in general)

Checklist

## Additional notes

For the record, it started with me adding a holiday calendar in my Home Assistant. I was left puzzled with that list:

Screenshot 2024-12-11 at 10-45-56 Paramètres – Home Assistant

And when I couldn't find the meaning of those abbreviations in the documentation either, I thought it was time to contribute.

The next step is to get in touch with the maintainers of this Home Assistant integration, and ask them to use the list of subdivision aliases when available.

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (c13eed0) to head (5362815).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #2162   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          194       194           
  Lines        11540     11541    +1     
  Branches      1641      1641           
=========================================
+ Hits         11540     11541    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

PPsyrius
PPsyrius previously approved these changes Dec 12, 2024
Copy link
Collaborator

@PPsyrius PPsyrius left a comment

Choose a reason for hiding this comment

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

LGTM 🇫🇷

I had to look up in the source code what MQ, RE... meant. The README
didn't help either, just listing the raw codes available.

Those are not of common use in the French culture, contrary to say, AZ
meaning Arizona in the US culture.

Also fixed the official spelling.
@bors-ltd
Copy link
Contributor Author

Sorry, I updated my PR after your review to write "La Réunion" everywhere for consistency.

That was also an opportunity to rebase on the upstream branch.

Copy link
Collaborator

@KJhellico KJhellico left a comment

Choose a reason for hiding this comment

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

LGTM.

@arkid15r arkid15r changed the title Document French subdivisions Add French subdivision aliases Dec 12, 2024
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

FR subdivs are still somewhat inconsistent with ISO 3166 but for v0 it's fine.

Thank you for adding this @bors-ltd

@arkid15r arkid15r enabled auto-merge December 12, 2024 17:08
@arkid15r arkid15r disabled auto-merge December 12, 2024 17:08
@arkid15r
Copy link
Collaborator

@bors-ltd FYI we started requiring signed commits this month however it's not checked on pre-commit or CI/CD so I'm accepting this one as is. Please look into setting this up if you're planning to contribute to holidays in future.

Thank you!

@arkid15r arkid15r merged commit 79638c2 into vacanza:dev Dec 12, 2024
30 checks passed
@bors-ltd
Copy link
Contributor Author

Thanks, I'll have a look at ISO 3166, and find out how to sign my commit if I have something to iterate on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants