Skip to content
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

Correct and enhance US holiday definitions #54

Merged
merged 5 commits into from
Jul 3, 2017
Merged

Correct and enhance US holiday definitions #54

merged 5 commits into from
Jul 3, 2017

Conversation

spaceneedle2019
Copy link
Contributor

@spaceneedle2019 spaceneedle2019 commented Jun 20, 2017

  • Add missing holiday information for a whole bunch of US states (now 54 in total!)
  • Correct some existing holiday information
  • Add custom functions for Georgia’s State Holiday, Lee-Jackson Day and Election Day
  • Add custom functions for Rosh Hashanah and Yom Kippur with pre-defined dates from 2014 to 2020
  • Add (missing) tests and correct existing tests
  • Add helpful comments
  • Add a lot of used sources

- Add missing holiday information for a whole bunch of US states
- Correct some existing holiday information
- Add custom functions for Georgia’s State Holiday, Lee-Jackson Day and Election Day
- Add custom functions for Rosh Hashanah and Yom Kippur with pre-defined dates from 2014 to 2020
- Add missing tests and correct existing tests
- Add helpful comments
- Add a lot of used sources
+ Add new method for Christmas Eve Holiday
Copy link
Member

@ttwo32 ttwo32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, LGTM.
But i want to hear @ppeble's thoughts.
@ppeble
Could you review this PR when you have a time?

Copy link
Member

@ppeble ppeble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I have questions about some custom methods but we can discuss them after I merge.

Would you mind maybe creating an issue that references this so I won't forget? Just link to the two holidays and say something like 'we would like this to be calculated so it will not break after 2020'. Simple is totally fine. 👍

rosh_hashanah:
arguments: year
source: |
rosh_hashanah_dates = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally don't mind custom functions but this seems like a time bomb that I will surely forget about by 2020. 😄

Is there a way to calculate Rosh Hashanah? Maybe we should add it into the available 'functions' over in rubyland.

That said, having it now is better than not having it so I'm going to totally accept this. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppeble:

Yeah, I agree with you. It's similar to Ramadan and Sacrifice Feast in the Turkish calendar (see #55). In addition to a calculation we need a transformation from the Hebrew into the Gregorian calendar to generate the dates for Rosh Hashanah and Yom Kippur.

https://en.wikipedia.org/wiki/Jewish_and_Israeli_holidays_2000–2050

yom_kippur:
arguments: year
source: |
yom_kippur_dates = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question about calculation. Is it possible to calculate Yom Kippur?

@ppeble ppeble merged commit 3755ca9 into holidays:master Jul 3, 2017
@spaceneedle2019 spaceneedle2019 deleted the enhance_us_holiday_definitions branch July 3, 2017 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants