-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
Albanian National Holidays #157
Albanian National Holidays #157
Conversation
- Add Albanian national holidays including tentative dates for islamic holidays that are TBD. - Test coverage.
- Add test coverage for holidays with standard dates.
- Add empty line in the end of the testing file.
- Fix empty line warning in the end of the file.
src/Countries/Albania.php
Outdated
'Dita e Pavarësisë' => '11-28', | ||
'Dita e Çlirimit' => '11-29', | ||
'Dita Kombëtare e Rinisë' => '12-08', | ||
'Krishtlindjet' => '12-25', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MirilTerolli Krishtlindjet
should be Krishtlindja
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fitimvata it's done.
In case you want to add multi language support: https://github.com/spatie/holidays/tree/main/lang |
- Translation for German, English, Spanish, French, Greek, Italian, Portughese, Russian, Turkish.
Good idea @Nielsvanpach , it's done! 👍 |
src/Countries/Albania.php
Outdated
2032 => '01-14', | ||
2033 => '01-02', | ||
2034 => '12-12', | ||
default => '01-01' // Temporary placeholder; requires ongoing maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would throw an exception by default and make sure we don't return false data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nielsvanpach , that would be a good idea, but this would prevent other standard holidays from not being displayed in case if we run something like this Holidays::for(country: "al", year: 2035)->get();
🤷♂️
src/Countries/Albania.php
Outdated
2032 => '03-22', | ||
2033 => '03-12', | ||
2034 => '03-01', | ||
default => '01-01' // Temporary placeholder; requires ongoing maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would throw an exception by default and make sure we don't return false data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nielsvanpach as mentioned above, throwing an exception will prevent the standard holidays from being displayed.
I wonder if this is what we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can't provide valid data, I prefer to throw an exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MirilTerolli I personally would do something like requested changes. Return null from getEidAlFitrHoliday
and getEidAlAdhaHoliday
and filter array in variableHolidays
src/Countries/Albania.php
Outdated
/** | ||
* | ||
*/ | ||
private function getEidAlFitrHoliday(int $year): string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private function getEidAlFitrHoliday(int $year): string | |
private function getEidAlFitrHoliday(int $year): ?string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fitimvata that could work as well, changes pushed.
src/Countries/Albania.php
Outdated
2032 => '01-14', | ||
2033 => '01-02', | ||
2034 => '12-12', | ||
default => '01-01' // Temporary placeholder; requires ongoing maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default => '01-01' // Temporary placeholder; requires ongoing maintenance. | |
default => null // Temporary placeholder; requires ongoing maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fitimvata pushed.
src/Countries/Albania.php
Outdated
}; | ||
} | ||
|
||
private function getEidAlAdhaHoliday(int $year): string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private function getEidAlAdhaHoliday(int $year): string | |
private function getEidAlAdhaHoliday(int $year): ?string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fitimvata changes pushed.
src/Countries/Albania.php
Outdated
2032 => '03-22', | ||
2033 => '03-12', | ||
2034 => '03-01', | ||
default => '01-01' // Temporary placeholder; requires ongoing maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default => '01-01' // Temporary placeholder; requires ongoing maintenance. | |
default => null // Temporary placeholder; requires ongoing maintenance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fitimvata pushed.
src/Countries/Albania.php
Outdated
/** @return array<string, CarbonImmutable|string> */ | ||
protected function variableHolidays(int $year): array | ||
{ | ||
return [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return [ | |
return array_filter([ |
src/Countries/Albania.php
Outdated
'E diela e Pashkëve Ortodokse' => $this->orthodoxEaster($year), | ||
'Dita e Bajramit të Madh' => $this->getEidAlFitrHoliday($year), | ||
'Dita e Kurban Bajramit' => $this->getEidAlAdhaHoliday($year), | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
]; | |
]); |
Official Albanian holidays, as documented by the Albanian government portal https://e-albania.al/Pages/OfficialHolidays.aspx
Includes tentative dates for
Eid al-Fitr
andEid al-Adha
holidays until2034
, because the dates are not static.