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 support for Slovenian holidays #48

Closed
wants to merge 95 commits into from
Closed

Add support for Slovenian holidays #48

wants to merge 95 commits into from

Conversation

juref
Copy link
Contributor

@juref juref commented Jan 17, 2024

Added Slovenian holidays

@juref
Copy link
Contributor Author

juref commented Jan 17, 2024

Maybe we could add a new method beside getName('Y-m-d') getTranslatedName('Y-m-d') to return the translated name. I see two good options:

Holidays::for('be')->getName('2024-01-01'); // Nieuwjaar - original
Holidays::for('be')->getTranslatedName('2024-01-01'); // New year - option 1
Holidays::for('be')->getName('2024-01-01')->translate('en'); // New year - option 2

In both options, we would need to add a translation file or reference array.

@juref juref changed the title Add Slovenia holidays Add support for Slovenian holidays Jan 17, 2024
Copy link

@nexxai nexxai left a comment

Choose a reason for hiding this comment

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

I believe the formatting for these dates should be "MM-DD" to match the rest

{
return array_merge([
'Novo leto' => '01-01', // New Year's Day
'Novo leto 2' => '02-01', // New Year's Day, yes it's a second day
Copy link

Choose a reason for hiding this comment

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

This should be 01-02 if the day is January 2nd, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for noticing. In out country the default is DD-MM that's why I accidentally put it this way.

},
{
"name": "Novo leto 2",
"date": "2024-02-01"
Copy link

Choose a reason for hiding this comment

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

Same here

@Nielsvanpach
Copy link
Member

Maybe we could add a new method beside getName('Y-m-d') getTranslatedName('Y-m-d') to return the translated name. I see two good options:

I'm open to accept a good PR which implements translations. #131

return 'si';
}

/** @return array<string, CarbonImmutable> */
Copy link
Member

Choose a reason for hiding this comment

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

can be removed

/** @return array<string, CarbonImmutable> */
protected function variableHolidays(int $year): array
{
$easter = CarbonImmutable::createFromTimestamp(easter_date($year))
Copy link
Member

Choose a reason for hiding this comment

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

use $this->easter($year) instead. Only available if your branch is rebased on the repo's main branch.

calonzolg and others added 26 commits January 19, 2024 17:45
* Add support for Irish public holidays

* Add logic that St Brigid's only started from 2023

* Add tests for St Brigid's Day

St Brigids Day is a new holiday in 2023 and it is variable so adding tests around the logic

* Apply PSR12 formatting
* adds croatian holidays

* stan fixes

* fixed allHolidays return type
* feature: add bolivian holidays

* test: add bolivia test

* test snap

* fixed typo and phpstan

* calculate correct easter date

* Fix styling

---------

Co-authored-by: rats4final <[email protected]>
Co-authored-by: rats4final <[email protected]>
* Add Estonian holidays

* Use easter() method for easter date
@Nielsvanpach
Copy link
Member

Something seem to went wrong with rebasing. Could you make a clean PR with only commits related to Slovenia? Thanks!

@juref
Copy link
Contributor Author

juref commented Jan 19, 2024

Sure, no problem.

@juref juref closed this Jan 19, 2024
@juref
Copy link
Contributor Author

juref commented Jan 19, 2024

New PR link:
#139

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.