implement missing holidays for ro region #126

[ste26054]

No description provided.

[ste26054]

For some reason, tests from travis seem to pass but this one is failing on my machine : https://github.com/holidays/definitions/pull/131/files#diff-e4db0e6f28a8a844e743514616f6cfefR87

It's like the range starting after 2017 is not considered:

2.4.4 :001 > Holidays.on(Date.civil(2017, 4, 17), :ro)
 => [{:date=>#<Date: 2017-04-17 ((2457861j,0s,0n),+0s,2299161j)>, :name=>"Paștele - luni", :regions=>[:ro]}] # Should be empty

[ppeble]

@ste26054 Unfortunately it is not straightforward to test branch/PR definitions in the actual ruby gem right now. This is due to the fact the ruby gem references this repository as a submodule and also because we perform a 'generate' step to create the final ruby classes that are actually used. This means that you need to jump through some hoops in order to perform a real test in ruby with your updated definitions. 😞

I do have an issue open to track this problem area here.

In your example above what steps did you take to get to that console? I created instructions on how to point the ruby repository to your branch definitions if you are interested. You don't have to do that if you aren't interested, though, as I will test things out myself right now. Only try out the instructions if you want to learn more about the ruby repo. 😄

[ste26054]

@ppeble I cloned the holidays repository then ran make clean-defs in it.
Because cloning via ssh is not possible from my network, I could not run BRANCH=improve_ro USER=ste26054 make point-to-defs-branch so I cloned my fork directly from the holidays repository, and pointed it to my branch improve_ro.
Then from holidays, I ran make generate then REGION=ro make test-region

[ste26054]

Actually I think there is an error in my test, as 2017-04-17 is a Monday, not a Friday...
I will correct that

[ste26054]

@ppeble Everything should be good now :)

[ttwo32]

LGTM.

[ppeble]

@ste26054 Oh good! Thank you for following up, I did try to look at it this last weekend but then ran out of steam and didn't have a chance to come back to it. 😄

Merging now! Thank you!