Make use of giggsey/locale opt-in

[MatTheCat]

While I understand the rationale behind #119 ,giggsey/locale dependency could lead to inconsistency for people which are already using the intl extension, and is useless for people which have up-to-date ICU.

As such I think you could add giggsey/locale (and ext-intl) in the suggest composer section and make the Locale class a variable (don't know how to do this properly though).

WDYT?

[giggsey]

It would have to be as part of a major version release (as the majority of people would just install libphonenumber-for-php, ignoring the suggestions, and wonder why the Geocoder doesn't work). I want to try to maintain the same version number as Google's libphonenumber, so I don't know when/if that'll happen.

I was surprised at how out of date certain operating systems are with the CLDR data. I remember the LTS version of Ubuntu was quite a bit out of date.

I agree that it's useless for people who have the intl extension, and up to date CLDR data, but the package is only 1.4MB and changes twice a year. As for people getting inconsistencies with the intl package, that'll be because the OS CLDR data is out of date, which ideally wouldn't happen.

[MatTheCat]

I want to try to maintain the same version number as Google's libphonenumber

Do you mean you cannot tag your own releases?

Anyways I can provide a PR in the eventuality the versioning scheme changes or Google releases libphonenumber 9. The rest will be up to you!

[giggsey]

Do you mean you cannot tag your own releases?

They've just released v8.8.10, so I'll match that version number.

So I'd need to wait until they release v9 before I introduce BC breakage.

Ideally, I'd use the intl extension if it was available, otherwise install giggsey/locale (maybe preferring giggsey/locale if its installed, as people might want up to date CLDR data). However, I'm not sure how to get that working with composer, and other projects that I've seen do it (I think symfony have it with their intl) cause issues when people update composer on one host with intl, but install on a different box without it.

[MatTheCat]

If giggsey/locale is only suggested then it should be used if the user requested it.

composer.json would look like this

"require": {
    "php": ">=5.3.2",
    "ext-mbstring": "*",
-    "giggsey/locale": "^1.2"
},
+"suggest": {
+        "ext-intl": "To get CLDR data from ICU",
+        "giggsey/locale": "To get up-to-date CLDR data"
+}

[giggsey]

Yes, but they need one or the other for the feature to work.

I'm not really comfortable making it a runtime exception if neither is available. We had that via #46, and one of the reasons for giggsey/locale was to avoid that potential exception.

[MatTheCat]

I think it's perfectly fine. The only case an exception would be thrown is if both intl and giggsey/locale are missing so the message would be something like "The intl extension is missing on your system, please require giggsey/locale."

[giggsey]

I don't want to reintroduce that runtime exception which can be avoided by a 1.4MB package.

Yes, it's an additional package, but it would be a waste to bundle it with libphonenumber-for-php as the two projects release on completely different cycles.

It'll also (for the majority of the time) contain more up to date & correct data than the intl extension installed on the OS.

[MatTheCat]

A runtime exception isn't a bad thing. Look at commerceguys/addressing for example. This is the best way to handle this type of situation.

[Warxcell]

Couldn't be that avoided with https://getcomposer.org/doc/04-schema.md#provide ?

Basically something like:

{
   "require": {
       "ext-intl": "*"
   }
}

And giggsey/locale:

{
   "provides": {
      "ext-intl": "*"
   }
}

I'm really not sure if that will work - just suggesting.

[stof]

that would be much worse, as giggsey/locale does not replace the full intl extension.