-
-
Notifications
You must be signed in to change notification settings - Fork 161
Canada provider: fix Canada Day definition and add National Day For Truth And Reconciliation #256
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
Conversation
add National Day For Truth And Reconciliation
src/Yasumi/Provider/Canada.php
Outdated
@@ -137,7 +138,8 @@ protected function calculateNationalIndigenousPeoplesDay(): void | |||
/** | |||
* Canada Day. | |||
* | |||
* @see https://en.wikipedia.org/wiki/Canada_Day | |||
* @see https://en.wikipedia.org/wiki/Canada_Day and Holidays Act, R.S.C., 1985, c. H-5 |
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.
If possible, can you add a link to that Holidays Act?
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.
added in a further commit
src/Yasumi/Provider/Canada.php
Outdated
@@ -253,4 +258,18 @@ private function calculateLabourDay(): void | |||
$this->locale | |||
)); | |||
} | |||
|
|||
private function calculateNationalDayForTruthAndReconciliation() |
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.
Do you have some sources that would provide a description of this holiday?
I looked at this page https://www.statutoryholidays.com/september30-holiday.php and according to that it is not a public holiday.
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.
References have been added to the phpdoc for Canada::calculateNationalDayForTruthAndReconciliation.
I do not know what the page to which you refer means by "public holiday" in a Canadian context.
If it means a day on which every employee in Canada is statutorily entitled to a day off with pay, then there are very few of those.
In Canada, constitutional authority to regulate employment is divided between the federal and provincial/territorial governments. Federally regulated workplaces observe federal statutory holidays. All other workplaces observe the statutory holidays of the province or territory in which they are located.
For a holiday to be one on which every employee in Canada is entitled to a day off with pay, it would have to be a statutory holiday federally and in every province/territory. It appears that is only true for New Year's Day, Canada Day, Labour Day and Christmas Day.
The page to which you refer correctly notes that National Day for Truth and Reconciliation is a federal statutory holiday. It is not a statutory holiday in any province/territory. So it is not a statutorily required day off with pay for every employee in Canada.
The Canada provider treats Good Friday, Victoria Day, Thanksgiving Day, Remembrance Day and Boxing Day as holidays. They are all federal statutory holidays. Each is also a statutory holiday in some but not all provinces/territories. Hence, they are not statutorily required days off with pay for every employee in Canada.
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.
Thank you for the extensive information. Public holidays being non-working days or not vary greatly between countries, and as such cause sometimes confusion. For this library I tend to only consider if a holiday is officially recognized by a local government. Irrespective if that means it is a non-working day or not. Because that is often arbitrary and not always regulated by a government.
This is one of the reasons why Yasumi is not perfect in this regards :) I am still looking into that how to do that better in a future release.
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.
So do you consider this a holiday by your definition?
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.
Yes, for this library any day can be included. Just a matter of the correct classification.
Thanks for the PR! Can you please update the CHANGELOG.md file reflecting the proposed changes/fixes? |
add National Day For Truth And Reconciliation
# Conflicts: # src/Yasumi/Provider/Canada.php # src/Yasumi/data/translations/truthAndReconciliationDay.php # tests/Canada/TruthAndReconciliationDayTest.php
add changes to CHANGELOG.md
changes added to CHANGELOG.md |
Should I squash my commits? |
You can if you like. I also can do it when merging your PR. Up to you :) |
Might be better if you squash when merging. |
@ovgray BTW Please change this PR to be merged into the |
I based my PR on the Master branch because I saw that PR#253, which added the Juneteenth holiday to the US provider, was based on and merged into the master branch (and, I note, is not yet in the develop branch). I will create a new branch based on the develop branch, add my code to it and do a fresh PR. |
@ovgray Ah yes I recall that accidentally I may have merged that Juneteenth feature in the master branch. Anyway, you should be able to change the base branch of a PR without creating a new one: https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request |
@stelgenhof I have provided a new PR #257 that makes the same changes to the develop branch, but with a TruthAndReconciliationDayTest that implements the new interface in the develop branch. I will close this PR in favour of #257. |
This PR fixes the definition of Canada Day in the Canada provider and adds National Day For Truth And Reconciliation