-
Notifications
You must be signed in to change notification settings - Fork 129
feat: add 2026-2027 holidays FR #531
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
base: develop
Are you sure you want to change the base?
Conversation
| '2023': [] | ||
| '2024': [] | ||
| '2025': [] | ||
| '2026': [] |
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.
Everything looks fine at a glance. Only this seems a little off to me. Empty arrays are fine if there are no holidays for Pont de l'Ascension. Did you remove them intentionally?
I would also have expected you to add something for 2026 or 2025 here and not for 2023 🤔
My French isn't good enough to check it myself in your sources.
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.
Empty arrays
Would you prefer if I put them back ? I used a fork we use internally (and empty arrays were removed) to updates those dates.
Checking dates
You can check every thing on the API that present data in a more developer ways ;)
If it's not clear enough, I can put it in another in order for you to check datas.
2023 date
I can remove it as well if you want
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.
Well, I would interpret an entry with an empty array to mean that there are no holidays in that year. A missing entry, on the other hand, can mean two things: 1. There are no holidays, or 2. No one has entered any holidays yet.
So if there are no holidays, I would prefer an empty array because it leaves no room for interpretation.
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.
Ok but there are holidays (for the "Corse" region), there are not in this file yet.
So following you, removing those empty array was the good choice no ?
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.
Correct. If there are holidays but the array is empty, that would be wrong. So the best option would be to fill the array, or the second-best option would be to remove it.
Re-opening of previous PR #530 (started over with a fresh fork that included the missing develop branch)
Adds school holiday dates for the 2025-2026 and 2026-2027 academic years to the official three-zone system (A, B, C).
Sources :
Changes
Following the API, it seems that "Pont de l'Ascension" was not correctly set up for 2025.
Note: I first tried Copilot to make these changes, but I had to check manually to make sure everything was correct (so I ended up doing it manually).