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

Provider for holidays in Latvia #70

Merged
merged 7 commits into from
Nov 13, 2017
Merged

Provider for holidays in Latvia #70

merged 7 commits into from
Nov 13, 2017

Conversation

lukosius
Copy link
Contributor

@lukosius lukosius commented Oct 16, 2017

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes

@lukosius lukosius changed the title Provider for holidays in Latvia [WIP] Provider for holidays in Latvia Oct 16, 2017
@stelgenhof
Copy link
Member

Thank you @lukosius for the PR! I'll have a look once you have added the unit tests. Without unit tests it would make it harder for me to verify and it ensures the quality of the library.

# Conflicts:
#	src/Yasumi/data/translations/christmasDay.php
#	src/Yasumi/data/translations/christmasEve.php
#	src/Yasumi/data/translations/easter.php
#	src/Yasumi/data/translations/easterMonday.php
#	src/Yasumi/data/translations/internationalWorkersDay.php
#	src/Yasumi/data/translations/newYearsDay.php
#	src/Yasumi/data/translations/secondChristmasDay.php
@lukosius lukosius changed the title [WIP] Provider for holidays in Latvia Provider for holidays in Latvia Oct 18, 2017
@lukosius
Copy link
Contributor Author

Added tests for the provider and some useful helpers.

Copy link
Member

@stelgenhof stelgenhof left a comment

Choose a reason for hiding this comment

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

Looking good!

*/
public function isWeekend(\DateTimeInterface $dateTime)
{
return in_array((int) $dateTime->format('w'), [0, 6], true);
Copy link
Member

Choose a reason for hiding this comment

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

Although in many countries weekend is considered to be Saturday and Sunday, in other countries it isn't. Need to be aware of that when using this function.

Copy link
Member

@stelgenhof stelgenhof left a comment

Choose a reason for hiding this comment

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

Sorry for the wait. All looking good!

@stelgenhof stelgenhof added this to the v1.7.0 milestone Nov 13, 2017
@stelgenhof stelgenhof merged commit 09341c6 into azuyalabs:master Nov 13, 2017
stelgenhof added a commit that referenced this pull request Dec 19, 2017
* Added Countable interface allowing proper of number of holidays is returned when using the count() function or ->count() method.

* Fixed styling.

* Adjusted unit test for Portugal so that the holiday of Corpus Christi is not validated between 2013 and 2016. Corpus Christi was not officially celebrated between 2013 and 2016.

* Provider for holidays in Latvia (#70)

* Add provider for official Latvia holidays
* Add translations for official Latvia holidays
* Add additional tests helpers
* Add tests for official Latvia holidays
* Update other tests using generateRandomDatesWithModifier
* Add default parameter to isWeekend to allow overwrite default weekend days

* Update CHANGELOG.md

* Implemented the Countable interface for all filters.

Signed-off-by: Sacha Telgenhof <[email protected]>

* Removed 'statehoodDay' from initial array as it is established at a particular year.

Signed-off-by: Sacha Telgenhof <[email protected]>

* Code cleanup.

Signed-off-by: Sacha Telgenhof <[email protected]>

* Sorted translations.

Signed-off-by: Sacha Telgenhof <[email protected]>

* Fixed test cases for Geneva as the holidays 'jeuneGenevois' and 'restaurationGenevoise' are established/restored at different moments in time.

Signed-off-by: Sacha Telgenhof <[email protected]>

* Updated for 1.7.0 release.

Signed-off-by: Sacha Telgenhof <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants