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

Add Indian holidays #3

Merged
merged 2 commits into from Sep 1, 2018

Conversation

notlmn
Copy link
Contributor

@notlmn notlmn commented Aug 26, 2018

More than 90% of holidays in India are dynamic. This PR adds only fixed holidays for now. Need to wait till #2 gets merged to add add dynamic holidays.

@coveralls
Copy link

coveralls commented Aug 26, 2018

Pull Request Test Coverage Report for Build 47

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 46: 0.0%
Covered Lines: 87
Relevant Lines: 87

💛 - Coveralls

@WebReflection
Copy link
Owner

latest version uses modules instead, where any date can be actually a function that will receive the meant year to deal with ... do you mid updating this PR reflecting that change ?

You can find an Italian easter example in here.

Or at least it could export a module so for the time being, your country has some date to show 😉

Thanks a lot for your initial PR anyway !!!

@WebReflection
Copy link
Owner

Apologies @notlmn but too many things changed in the meanwhile (early state, I want to get this right).

Please have a look at latest state and see the holidays/.exmple folder to know more.

Feel free to check the utils.js file too and other countries to see how things are retrieved (i.e. the gb or the us are quite complete in potentials)

@notlmn
Copy link
Contributor Author

notlmn commented Aug 27, 2018

Hey, I am working on it. I also did a rebase locally. This PR should not affect most things considering the changes are in a separate file.

I waited because I was working on how to find dates for holidays, because holidays in India literally depend the position of Sun and Moon.

@WebReflection
Copy link
Owner

wow! looking forward to the updated PR then 🎉

@WebReflection WebReflection reopened this Aug 27, 2018
@WebReflection
Copy link
Owner

P.S. holy sh.... https://www.officeholidays.com/countries/india/index.php 😱

Please remember to use regional array for all holidays that are not national, thanks!

@notlmn
Copy link
Contributor Author

notlmn commented Aug 27, 2018

Yeah, holy whatever.

As I told you already more than 90% of holidays are dynamic (most literally depend on position of Sun and Moon, and most of them regional). The one's already statically provided are the only national holidays.

I'm thankful for your patience, because this PR might take some more time because I'm in an overcrowded train that might take another 2 hours to reach it's destination.

@WebReflection
Copy link
Owner

I didn't mean to rush you at all ... take all the time you want but I'm sure already it'd be a fantastic contribution (in general, not only for this project).

@notlmn
Copy link
Contributor Author

notlmn commented Sep 1, 2018

@WebReflection I've rebased and added all the static holidays for now.

It looks like the dynamic holidays might take some more time (I'm planning to work on an Indian calendar that might help solve dynamic dates).

You can merge this for now, I'll make a PR later to remaining dynamic holidays when I'm done.

@WebReflection WebReflection merged commit 4736057 into WebReflection:master Sep 1, 2018
@notlmn notlmn deleted the add-indian-holidays branch September 1, 2018 15:55
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.

None yet

3 participants