search

azuyalabs / yasumi

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

Allow default locale to be configured #123

Closed c960657 closed 4 years ago

c960657 commented 5 years ago

In preparation for the removal of $this->holiday from Holiday and Provider #122, this adds the suggested default language setting in backwards compatible way.

Two new class methods are added, Yasumi::setDefaultLocale($locale) and Yasumi::getDefaultLocale(), and a new optional argument is added to $holiday->getName($locale = null) that allows overriding the default.

This approach is used e.g. by the PHP intl extension, ~the Symfony Translator~, Laravel, Drupal's t() function and the Punic library.

Passing a locale to Yasumi::create() or Yasumi:: createByISO3166_2() is now deprecated (but still works as before). People can already now migrate to the new behaviour suggested for Yasumi 3.0 in #122. A deprecation warning is triggered following the tradition in Symfony.

This paves the way for a second PR (#124) that adds support for fallback locales. I made two PR's in order to make code review easier. I recommend merging both PR's before doing a new release to ensure that the dust has settled on the API.

c960657 commented 5 years ago

@stelgenhof What do you think of this?

For the record: This does not break backwards compatibility but only deprecates some things. Such deprecations should preferably land early in the 2.x development cycle to allow users of the library to adjust their code already now, so their eventual upgrade to 3.x will be seamless.

c960657 commented 5 years ago

@Milamber33 The visibility changes are not part of this PR.

They were committed to the develop branch back in January 2019. 326e8ecfd5a7162d0a999ebfbe2f86368cc56e08 22609ce0fa3a643fdeaa237d0834be3fd1538e44 d7b5e9274923bbb9bc3cd989bce350918224f327 13dd203fb9356cfaae58ce8f3bb994f2def38d0e f1a4892c5cdebe058cf5f1fe5fa7e9024649d489

Milamber33 commented 5 years ago

@c960657 Okay, noted, I missed that. The question remains, though I grant that this is probably the wrong place for it now.

stelgenhof commented 5 years ago

@Milamber33 As for the visibility of class members, you are right. When I started the project I didn't pay attention to the visibility much (I know: bad me!), however this should be changed of course.

stelgenhof commented 5 years ago

@c960657 I had a look at the code of this PR and think it looks good. Just want to do a few more test rounds.

My plan is do to a 2.2 release with this PR included. 2.2 will also contain all changes since v2.1 plus I am working on the Chile holiday provider. With this, also the documentation needs to be updated.

I don't have a particular target date yet in mind, but hopefully soon.

BTW: Big thanks for the coffee!! I feel I should return the favor as you have contributed much to Yasumi :)

Cheers! Sacha

c960657 commented 5 years ago

There is something about this that does not feel right. I seem to mix up default locale and fallback locale. I'll prepare an update. Stay tuned. I suggest this is merged after #176 (if that is accepted).