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 adr template and for adr for empty subdivisions #465

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Conversation

derTobsch
Copy link
Contributor

closes #430

@derTobsch derTobsch added this to the 0.26.0 milestone Feb 20, 2024
@derTobsch
Copy link
Contributor Author

@XSpielinbox if you want you could take a look at this adr :)

Copy link

sonarcloud bot commented Feb 20, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@XSpielinbox
Copy link
Contributor

@XSpielinbox if you want you could take a look at this adr :)

Thank you! I definitely will take a closer look as soon as I have time.

@derTobsch derTobsch added topic: documentation Improvements or additions to documentation type: enhancement A general enhancement labels Feb 23, 2024
@derTobsch
Copy link
Contributor Author

@XSpielinbox if you want you could take a look at this adr :)

Thank you! I definitely will take a closer look as soon as I have time.

Hey @XSpielinbox, i would like to merge this to gain progress.
If there is anything missing or wrong in the documentation, please let me know and we fix it afterwards :-)

@derTobsch derTobsch merged commit b62b67d into main Feb 23, 2024
10 checks passed
@derTobsch derTobsch deleted the 430-adr branch February 23, 2024 15:06
@derTobsch derTobsch removed the type: enhancement A general enhancement label Feb 23, 2024
@XSpielinbox
Copy link
Contributor

XSpielinbox commented Mar 7, 2024

So, finally:
Sorry, for the long delay:

  1. The Context of ADR 001 is a bit brief in my opinion, but should be fine.
  2. There are multiple locations where regions can be defined (the .xml files, the HolidayCalendar Enum, the .properties files). Perhaps it would be wise to clarify when a country/region code should be added where (and even if it shall be added everywhere at once, this would make sense to me to ensure that one does not forget / overlook one of the locations). One could also take this as an opportunity to clarify what consequence/use each of those entries has (e.g. the description in the .xml vs. the country_descriptions.properties).
  3. The city/regional holiday handling as discussed in Improve india and moldova for ISO3166-2 #360, Assumption day is a holiday in most of bavaria #269 and Breaking: make country codes ISO 3166 conformant #267 should definitely be an ADR in my opinion.
  4. I would appreciate an ADR on how to handle unreliable data like the date of New Year's Day and 2nd January or Christmas Day and Boxing Day in Scotland, as mentioned in Fix United Kingdom of Great Britain and Northern Ireland #353 or the date of Christmas Day and Boxing Day in Bermuda as discussed in Fix British Overseas Territories #342, unclear start years or the holidays in the Bahamas (see Add Bahamas public holidays #277)
  5. Adding of sources as discussed in Add sources to holiday.xsd #332 should be written down in an ADR.
  6. Usage of xinclude as you proposed in https://github.com/focus-shift/jollyday/pull/335/files#r1342750983 should be discussed in an ADR.
  7. The adoption of ISO 3166 should be outlined in an ADR, see Breaking: make country codes ISO 3166 conformant #267 and Breaking: Move British Crown dependencies to own countries #326
  8. Some holidays do not follow a regular schedule, so they have to be hardcoded, of course. But some others do, but the respective regularity is (currently) just not supported by jollyday. How to then handle these holidays should in my opinion be clarified in an ADR as well. See Add Mauritius public holidays #279, Add Singapore public holidays #283 and Add Hong Kong public holidays #288

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Describe Development Decisions in the Readme
2 participants