best way of configuring existing and new holidays? about holidays

[scottsemple]

This is related, but please let me know if I should start a new thread.

In order to adjust the us.yml file for the holidays that are observed by my company, I did the following:

• Forked the gem (https://github.com/harvesthq/holidays);
• Created a harvest.yml file (https://github.com/harvesthq/holidays/blob/master/data/harvest.yaml);
• Edited the harvest.yml file contents and tests;
• Edited the index.yaml file to include HARVEST: ['harvest.yaml']; and
• Ran rake generate and rake test to generate the necessary Ruby files. Both ran successfully.

I then ran my test suite and the tests that were previously passing with date.holiday?(:us) are now failing on date.holiday?(:harvest). The error message is Holidays::UnknownRegionError:Could not load holidays/harvest.

Is there another file where I should be adding :harvest as a region? Thanks!

[scottsemple]

Have you had a chance to look at this, @alexdunae?

Is there something else that I should be doing here? Thanks!

[alexdunae]

Finally chiming in to let you know this isn't totally being ignored. Adding holidays at runtime is definitely something that needs to happen without needing to fork.

Regarding your is @scottsemple, I loaded up your yaml file and added to my index.yaml, as you did, and got:

~/Code/holidays (master) $ irb
irb(main):001:0> require_relative 'lib/holidays.rb'
=> true
irb(main):002:0> Date.civil(2007,1,1).holidays(:harvest)
=> [{:date=>#<Date: 2007-01-01 ((2454102j,0s,0n),+0s,2299161j)>, :name=>"New Year's Day", :regions=>[:us, :harvest]}]
irb(main):003:0>

Are you still having issues with it?

[ppeble]

I have a PR allows for the loading of holiday definitions at runtime.

I'm no longer totally sold that the solution in the PR is the right overall direction but it might be a good stopgap to offer the functionality until we decide on a final path.

[ppeble]

I have merged the PR I mentioned, which allows for the loading of custom holidays on the fly. @pkananen and @scottsemple, does this address your use case sufficiently? Can we go ahead and close this issue?

[pkananen]

I think we can.