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

Add JE (PH) #424

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Add JE (PH) #424

wants to merge 2 commits into from

Conversation

Lee-Carre
Copy link

Preliminary addition of PH for (the island / Bailiwick of) Jersey. Closes #397.

SH (#398) is proving a bit trickier, so still working on that set. Doing it all on a phone is proving to be somewhat of a challenge.

First PR, so any guidance is appreciated.

Added Public Holidays specification for Jersey.
Updated /src/holidays/index.js to include entry pointing to new je.yaml
@Lee-Carre
Copy link
Author

Given #415:

  • I've included the Diamond Jubilee holiday, as a 1-off (for-only-that-year) entry, since it apparently applies beyond the UK (The Bailiwick of Jersey is a crown dependency, not part of the UK (or, technically, Britain), but under the crown).
  • I, too, wasn't sure of the best way to handle substitute holidays, so added them as fixed_date entries (with obvious names) per-year.

@ypid
Copy link
Member

ypid commented Dec 30, 2021

PH that is not every year cannot be expressed here. I don’t want to extend the holiday code and better solve #300 some day. https://github.com/KDE/kholidays supports this which is the current favorite (#300). You would have to drop this from this PR. You can consider submitting to https://github.com/KDE/kholidays to not lose this info if you care.

substitute holidays

Search for nextSaturday20Jun in https://github.com/opening-hours/opening_hours.js/blob/master/src/index.js

Doing it all on a phone is proving to be somewhat of a challenge.

Thanks for your effort!

@Lee-Carre
Copy link
Author

Thanks for the prompt review.

I'll return to this & make refinements next week once all the festive distraction is over.

However, for now:

PH that is not every year cannot be expressed here.

You would have to drop this from this PR.

I wasn't aware of this. The documentation (/src/holidays/README.md) is misleading, and should be updated.

@ypid
Copy link
Member

ypid commented Jan 2, 2022

You are welcome.

I wasn't aware of this. The documentation (/src/holidays/README.md) is misleading, and should be updated.

I am not sure about that. You used 2020 as key:

PH: # http://www.gov.je/leisure/events/whatson/pages/bankholidaydates.aspx
 - {'name': '75th Anniversary of VE Day', '2020': [5, 8]}

The allowed keys are documented here: https://github.com/opening-hours/opening_hours.js/tree/master/src/holidays#holiday-definition-format-ph

What can be confusing is the difference between PH and SH. They have different formats. Year is only supported for SH.

@Lee-Carre Lee-Carre marked this pull request as draft February 24, 2022 17:17
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.

PH for Jersey
2 participants