Add missing market calendars#200
Conversation
|
Ok, I am calling a timeout here. I do not have time to QA all of this. Over in Rcpp I have this to say: Contributing. Please switch to sending PRs only after filing an issue and both of us agreeing that it is the right step. I do not have time to check each new calendar for the QL version that brought it, and doing it unchecked risks breaking with random old QL installation as I encounter on CRAN machines. |
|
No problem, will create issues instead of PRs in the future. |
|
Just the Poland change may be sensible if fully guarded. The PR just looked to big with the "/" and "::" stuff. I think going forward sticking with just '/' is fine. Over on qlcal which has more of an R focus we can be cute and also support '::'. But let's focus on what matters and the dance around adding '::' everywhere may not pass that test. If if you minimise this PR to just Poland it is a little easier to take (especially as you version guarded that). Note that there is also file |
|
The |
|
Shall we resurrect this one and reduce it to just the Poland change? |
sure will create a new PR |
|
It's up to you. I can also just copy the few lines. When I used the term 'resurrect' I had a literal reopening of this PR in mind, and tuning its code change down to a smaller subset. I did not ask for a new PR for just three lines of code -- not really worth your or my time. In better news, I am in good standing again at CRAN as I got Simon to update QuantLib on macOS. That fixed things. No new upload needed. |
|
github didn't let me reopen, since I've rebased ... |
Hm. Odd. I don't see why a rebase would matter but I concur, the button is greyed out for me too, in my own repo. Oh well. |
2 participants
the poland calendars are version gated on QuantLib ≥ 1.36.0