azuyalabs / yasumi

The easy PHP Library for calculating holidays
https://www.yasumi.dev
Other
1.04k stars 152 forks source link

Brazilian Carnival #90

Closed glauberm closed 6 years ago

glauberm commented 6 years ago

I believe there is an error in the calculation for the Brazilian Carnival holiday days.

Since #36 it is subtracting 51 days from Easter Day, but, although the celebrations starts then, the holiday itself is officially in the Tuesday 47 days before Easter.

The Ash Wednesday comes right after that and is also not included.

I couldn't find a lot of good sources for this in English, but take a look at the U.S. Embassy in Brazil website.

Additionally, the Monday before Carnival is usually a holiday too. Should I include that in my PR?

stelgenhof commented 6 years ago

Thank you very much for spotting this.

The holidays in Yasumi should be 'official', meaning endorsed or recognized by the local government. Just to understand, do you think that the changes made in #36 shouldn't have been done?

If the Monday before Carnival is related to the Carnival holiday, then yes you can include it in the PR.

Thanks again for your help!

glauberm commented 6 years ago

Just to understand, do you think that the changes made in #36 shouldn't have been done?

Yes.

Here you can see a list of national holidays in our Ministry of Planning website. It doesn't get any more official than that.

Expect my PR in a couple of days.

stelgenhof commented 6 years ago

Thanks for the update! Looking forward to your PR :)

glauberm commented 6 years ago

92 Where is Scrutinizer config file?

stelgenhof commented 6 years ago

@glauberm Oops! My bad, I forgot to add one. I will do that right away.

glauberm commented 6 years ago

92 Scrutinizer check is being canceled with error Code coverage data is not yet available. I'm not familiar with it, it's something that is fixed automatically or should I make another push tomorrow?

stelgenhof commented 6 years ago

Should be fine now. I accidentally included an option for external code coverage check. It's disabled. Can you give it another try? Thanks!

glauberm commented 6 years ago

92 All checks have passed. :100:

stelgenhof commented 6 years ago

Closing this issue (Resolved by PR #92)