Hey!

Thanks for your help, and sorry for the late reply.

Just a couple of things I noticed after testing locally.

• When we are checking for the holidays (ie via Holidays.on(Date.civil(2008, 4, 25), :cn)), it runs Holidays::DateCalculator::LunarDate in the Holidays repo. Here it requires that there is a map for the Chinese Lunar calendar. Is this something you added while testing locally?
• Since we are adding another country definition, do we need to update the tests in Holidays, namely around the number of definition files it expects?

Hi!

I used lunar_to_solar, as in kr, Is this not correct?

I ran the rake test command on this PR and the tests came back green. I don't know what else needs to be updated. (apologies, I can't run this again; I had to delete a bunch of things locally to clear space)

Hey,

After investigating further, I think you can close this PR, due to the somewhat complicated nature of the Chinese holidays. Some changes would need to be made inside the Holidays repo for it to work correctly as well; it currently doesn't deal with lunar dates correctly in some cases.

Thanks for your help!

What are the next steps? Update the built-in method? I could just pre-compute these, as was done with vernal_equinox. Or we could just roll with a subset of the holidays based on solar calendar, and add those in with another PR.

Maybe it is better to update the code to support holidays from more regions? In addition to Chinese holidays, Islamic holidays and Hindu holidays such as Diwali would be nice additions.

I am going to close this just because of the inactivity. I think this is a worthy thing to add....but unfortunately it's simply not easy to do with the current setup. We need a more comprehensive solution to holidays in other date formulations. Please post in this thread if you disagree and I will be happy to reopen to discuss further.