search

azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.04k stars 152 forks source link

Canada provider: fix Canada Day definition and add National Day For Truth And Reconciliation #256

Closed ovgray closed 3 years ago

ovgray commented 3 years ago

This PR fixes the definition of Canada Day in the Canada provider and adds National Day For Truth And Reconciliation

stelgenhof commented 3 years ago

Thanks for the PR! Can you please update the CHANGELOG.md file reflecting the proposed changes/fixes?

ovgray commented 3 years ago

changes added to CHANGELOG.md

ovgray commented 3 years ago

Should I squash my commits?

stelgenhof commented 3 years ago

Should I squash my commits?

You can if you like. I also can do it when merging your PR. Up to you :)

ovgray commented 3 years ago

Might be better if you squash when merging.

stelgenhof commented 3 years ago

@ovgray BTW Please change this PR to be merged into the develop' branch and not themaster`. Thank you!

ovgray commented 3 years ago

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.

stelgenhof commented 3 years ago

@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

ovgray commented 3 years ago

@stelgenhof I think the develop branch has too many changes from the master branch for that to work. For one thing, tests in the two branches implement different interfaces. The TruthAndReconciliationDayTest in this PR would have to be edited to be testable in the develop branch, and then would no longer be testable at my end.

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.