-
-
Notifications
You must be signed in to change notification settings - Fork 23
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 Mauritius public holidays #279
Conversation
Good Morning @XSpielinbox, could you rebase your branch against the main? And maybe split the other pull requests #283 and #288 into branches? That would be easier to merge and maintain. |
Good morning @derTobsch, for sure I can rebase against main. I will have to see how to resolve the conflicts in the holiday_descriptions.properties. What do you mean with "split the other pull requests [...] into branches"? I can rebase them against my respectively rebased branches for this PR and #283 or main again too, as they are on separate branches already, but I can only open pull-requests against existing branches in this repository. |
b3caa57
to
d4a065c
Compare
b069b2f
to
76c6c52
Compare
Can you give me an example? And if both are not given, then it should not throw an exception, because this is legit if there was not starting or ending year of a public holiday. |
I added a test case for that, see FixedParserTest.java. I noticed this shortcoming in the making of the previous holidays already (with that fix, some things in the already merged holidays could be solved more elegantly), but with Mauritius I saw no way around this anymore. The issue is that in case of a "only every x years" cycle with only a
Yes, of course it is legitimate to have no start or end year. I did not change that. After all all the other holidays work as before and all the tests still pass. The problem is that for holidays that only occur every x years with x > 1, I don't know a way to determine which cycle is ment without any additional information. If I say every year, it is trivial. If I say every odd or every even year I also don't need a start or end date to say whether this year is in the cycle or not. But when I say every 4 years for example, how do I know whether the years should be ..., 2012, 2016, 2020, ... or ..., 2013, 2017, 2021, ... or 2014, 2018, 2022, ... or 2015, 2019, 2023, ... If I state |
8cbd253
to
6ab4429
Compare
Signed-off-by: XSpielinbox <[email protected]>
@XSpielinbox can you translate it with https://www.deepl.com/translator for the other languages? |
Hey @XSpielinbox, I improved the code a little bit to clear things up and added tests for the ValidCycle class. |
86ae031
to
960a565
Compare
For sure, I can do that. However, you merged it already. Shall I just make a new PR for that? |
Hey, now I know what I forgot. I wanted to write you that the translations are done. I translated most of the new holidays. I need to look after the last translations, but that is not a big deal. There where some issues with the encoding that I just fixed. So now that should be ok for the moment |
Ah, ok. Thank you. Yes, the German and Greek translations seem finished. I noticed a typo in the German translation however: It should be "Tag der Unabhängigkeit und der Republik". The French, Dutch, Portuguese and Swedish translation are still completely missing, however. Shall I add them or will you do that too? |
I will do that. I want to take a deeper look into the translations. And maybe provide some header for the utf8 encoding if that is possible |
Ok, but about the changes you made to the country_descriptions_nl.properties:
An explenation that seems very relevant and good in this context: https://stackoverflow.com/questions/4659929/how-to-use-utf-8-in-resource-properties-with-resourcebundle -> as far as I understand it, there is no such thing as a "header" for UTF-8 encoding in .properties files. For legacy reason an awful encoding is assumed by default in some tooling, especially old versions. Newer versions should default to UTF-8 and everything should be fine, but maybe one has to adjust some things to not use the legacy defaults (thereby I mean the IDE configuration or the java code that handles these files, though since Java 9 that should be fine too). |
@derTobsch Sorry, I by accident synced the wrong branch of my fork with upstream (which I just reverted) wherefore this PR is not successfully merged and closed anymore, but only closed. I committed the typo fixes to this same branch. You may cherry-pick them or I can open a second PR. If you want I can also migrate them to a new branch and open a new PR from there. |
🎁 Adds support for Mauritius
🐞 fix Cycle definitions for when they do not contain a
validFrom
definition. Previously it would just silently fall back toEVERY_YEAR
behavior. Now it calculates backwards, if only avalidTo
is given and throw an exception if neither is given.However, it is missing translations at the moment, as I did not know how to translate them.
Also as long as there is no support for the Tamil and/or Hindu Calendar (see #282), one has to manually define many holidays, which is very time consuming and error prone.
Additionally as long as there is no support for the Chinese Calendar (see #285), one has to manually define many holidays, which is very time consuming and error prone.
Please merge this PR AFTER #277.