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

UK Early May Day 2020 moved #37

Merged
merged 1 commit into from
Jun 8, 2019
Merged

UK Early May Day 2020 moved #37

merged 1 commit into from
Jun 8, 2019

Conversation

skipishere
Copy link

This fixes the Early May Day Bank Holiday to be on the 8th May due to the 75th anniversary of VE day.
Note that as at this time this applies to England, Wales and NI but Scotland is yet to approve this (but is widely expected to, and I can't see where Scottish Bank Holidays are defined, as they are different)

@skipishere
Copy link
Author

#37

@martinjw martinjw merged commit 4d5ac92 into martinjw:master Jun 8, 2019
@martinjw
Copy link
Owner

martinjw commented Jun 8, 2019

Thanks!
There's no Scotland holidays yet- I think the legal responsibility is local authorities, not even Holyrood, which makes it even more complicated (and the banks follow rest of UK anyway).

@skipishere
Copy link
Author

No worries, figured I'd try my first pull request on a nice easy change :)

Only mentioned the Scotland thing as I know they have the 2nd working day at New Years as Bank Holiday that the rest of the UK don't (but couldn't see any logic for it) & didn't want to break anything

@martinjw
Copy link
Owner

martinjw commented Jun 8, 2019

Congratulations on your first PR!

@crocom
Copy link

crocom commented Jun 17, 2019

Hi we are using you library v. helpful, but I note that in the logic for IsBankHoliday you assume ones in May are always on a Monday, but VE Day 2020 is on Friday hence new UKBankHoliday().IsBankHoliday(new DateTime(2020, 5, 8)) returns false ...

@martinjw
Copy link
Owner

Yes, I'll fix that... Thanks for comment.

martinjw pushed a commit that referenced this pull request Jun 17, 2019
@crocom
Copy link

crocom commented Jun 19, 2019

Thanks for fixing that so quickly.

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.

3 participants