remove valentine's day as a spanish holiday

azuyalabs / yasumi

The easy PHP Library for calculating holidays

https://www.yasumi.dev

Other

1.05k stars 155 forks source link

remove valentine's day as a spanish holiday #204

Closed dretamal closed 4 years ago

dretamal commented 4 years ago

Valentine's day has never been a spanish holiday.

It is a working day in every region in the whole country

stelgenhof commented 4 years ago

It's correct that Valentine's Day is not a holiday in the true sense, i.e. being a non-working day, as it is in many countries.

Unfortunately this shows some weakness of Yasumi and that it cannot distinguish holidays to be non-working day. See also #99 for a discussion about this.

I wouldn't necessarily remove this holiday, but rather need to think of a solution how we can classify these situations.

Cheers! Sacha

github-actions[bot] commented 4 years ago

This pull request has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.

mgarciadelojo commented 4 years ago

What else is it needed for this PR to be merged?

@stelgenhof as @dretamal says, Saint Vallentine's has never been a bank holiday in Spain, which is what this library is about, right?

stelgenhof commented 4 years ago

@mgarciadelojo Please see my previous comment. The issue isn't really about whether Valentine's Day is an officially recognized holiday or not. For this library, I'd like any day to be able to be added from official ones (first priority) to common observed events.

Valentine's Day doesn't necessarily need to be removed, but unfortunately it is marked as non-working day by the API which is a bit unfortunate. Which isn't a particular Valentine's Day issue but rather an API deficit.

mgarciadelojo commented 4 years ago

So what you are saying is that if we use the isWorkingDay function, it should be resolved?

stelgenhof commented 4 years ago

@mgarciadelojo Unfortunately not. There isn't any solution within the library at the moment unless some changes are made. The library considers a working day a day that doesn't fall in the weekend or is a registered holiday in the provider. For cases like Valentine's day this isn't sufficient and hence needs to be changed.

If you can't wait for changes in this library, what you could do is:

Exclude Valentine's day as a working day at your end (in your application's logic)
Fork this library and remove Valentine's day from the list of holidays in Spain.

I'd like to make changes to Yasumi so it can properly set whether a holiday is a working day or not, however don't want to rush into things. It should be done properly which takes some time to think about and implement.

Cheers! Sacha

mgarciadelojo commented 4 years ago

It makes complete sense. Thanks a lot for the explanation @stelgenhof

github-actions[bot] commented 4 years ago

This pull request has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.

github-actions[bot] commented 4 years ago

This pull request has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.

github-actions[bot] commented 4 years ago

This pull request has been open 60 days with no activity. Please remove the stale label or comment, or this will be closed in 10 days.