-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
New Zealand calendar missing Matariki holiday #1564
New Zealand calendar missing Matariki holiday #1564
Conversation
Updated the header file with Doxygen tags and with the link to the NZ government website
Thanks for opening this pull request! It might take a while before we look at it, so don't worry if there seems to be no feedback. We'll get to it. |
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.
Thanks! It needs a couple of fixes to compile (see comments) but it looks ok.
ql/time/calendars/newzealand.cpp
Outdated
@@ -62,6 +62,26 @@ namespace QuantLib { | |||
// Boxing Day, December 26th (possibly Monday or Tuesday) | |||
|| ((d == 26 || (d == 28 && (w == Monday || w == Tuesday))) | |||
&& m == December)) |
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.
you need to remove the last )
here...
ql/time/calendars/newzealand.cpp
Outdated
|| (d == 14 && m == July && (y == 2023 || y == 2028)) | ||
|| (d == 15 && m == July && (y == 2039 || y == 2050)) | ||
|| (d == 18 && m == July && y == 2036) | ||
|| (d == 19 && m == July && (y == 2041 || y == 2047)) |
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.
...and add it here at the end.
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.
Arrgh... Thanks Luigi and sorry for the loss of time! Ok noted: before pushing any PR to master (even those that look very simple) I have to code it and be sure that it compiles successfully in my branch locally. Only after local branch build and test is successful, then I will push it to the main branch.
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.
No problem 😄
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.
Thanks Luigi! This time after coding I built the branch and it was ok. Tested the behaviour of all the 31 dates and it looks good. I attach the test file. Have a nice evening.
NZMatariki.cpp.txt
Congratulations on your first merged pull request! |
Updated the header file with Doxygen tags and with the link to the NZ government website. Updated the implementation with the Matariki dates. Instead of blindly adding 31 dates I tried to optimize the code as much as possible. Since I am a newbie I am not sure if the coding logic can be optimized further. Thanks in advance for your comments/review.
Matariki_NZ.ods