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
Update South Korea holidays, add l10n support #1536
Conversation
Pull Request Test Coverage Report for Build 6793343944Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
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.
@PPsyrius, it's just great!
Turns out #1535's new We may as well go and revamp the existing codes for |
I'm trying to migrate South Korea to ObservedHolidays, but I'm not satisfied with the result yet. Their rules are quite complex and unique. |
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.
Smooth migration plus l10n, 2 in 1!
A few suggestions:
Co-authored-by: Arkadii Yakovets <ark@cho.red>
Kudos, SonarCloud Quality Gate passed! |
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.
🇰🇷 LGTM
Proposed change
en_US
,kr
,th
localization).Article 34 of the Public Official Election Act
).Temporary Public Holidays
from 1948-2023 as part of theStaticHolidays
class.Public Holidays Act
.Type of change
Checklist
make pre-commit
command generates no changesmake test
,make tox
(we strongly encourage adding tests to your code)