-
Notifications
You must be signed in to change notification settings - Fork 473
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 FI: add unofficial holidays #1885
Update FI: add unofficial holidays #1885
Conversation
2e7204e
to
9c3562a
Compare
Current implementation generates only list of Public holidays (so holidays established by Parliament). However there are also quite a lot of so called "flag flying days" ("liputuspäivät" in Finnish or "flaggdagar" in Swedish) that don't have official holiday status, but still it's quite common for businesses to have shorter opening hours or to be completely closed. So this adds a list of those holidays under unofficial category. The English and Swedish translations are taken from the Ministry of the Interior website, and for Ukrainian one I asked a Ukrainian friend of mine (who's native language is Ukrainian) to review the list for me. Sources: - https://en.wikipedia.org/wiki/Flag_flying_days_in_Finland - https://intermin.fi/en/flag-and-arms/flag-flying-days (Ministry of the Interior of Finland)
9c3562a
to
56d6373
Compare
Sonar cloud is apparently not happy with all the year checks in |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1885 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 185 185
Lines 11222 11263 +41
Branches 1766 1785 +19
=========================================
+ Hits 11222 11263 +41 ☔ View full report in Codecov by Sentry. |
@alexei-mobal, it's great contribution! I am not very familiar with Finnish traditions and holidays, so I have a few questions here.
|
Mm, good point! I initially used a mix of holidays mentioned on the wikipedia page and ministry website, but maybe it's better idea to treat information provided by the ministry as single source of truth 👍
Same as above, I guess I ended up using a mix of names from wikipedia and the ministry website, will use the latter everywhere. |
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.
Thanks for the PR, please take a look at these comments when you get a chance:
Just ignore it, consider it as FYI message. |
As requested in PR by arkid15r.
Ok, @KJhellico @arkid15r I updated PR with all your suggestions; now we don't list recommended flag days (there was only one in first version of this PR), and all names should follow the ones suggested by Finnish Ministry of Interior (with the exception of Ukrainian ones -- those are produced by combined forces of Google translate & my friend 😅 ). Let me know if I missed something. One last thing I'd do is rebase this on top of latest |
...that I missed during previous inlining.
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 👍
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, just a few minor suggestions/comments:
Quality Gate passedIssues Measures |
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
@KJhellico any last minute suggestions?
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.
@alexei-mobal thank you for this contribution and the clean/tested code!
It's great to see another community member efforts for holidays coverage improvement 👍
@arkid15r you're welcome, happy to contribute! 🙏 And thanks to you and @KJhellico for all the review comments, learned a thing or two myself, I'm not very savvy in python yet :) |
Proposed change
Current implementation generates only list of Public holidays (so
holidays established by Parliament). However there are also quite a lot
of so called "flag flying days" ("liputuspäivät" in Finnish or "flaggdagar"
in Swedish) that don't have official holiday status, but still it's
quite common for businesses to have shorter opening hours or to be
completely closed.
So this adds a list of those holidays under unofficial category.
The English and Swedish translations are taken from the
Ministry of the Interior website, and for Ukrainian one I asked a
Ukrainian friend of mine (who's native language is Ukrainian) to
review the list for me.
Sources:
Interior of Finland)
Type of change
Checklist
make pre-commit
, it didn't generate any changesmake test
, all tests passed locally