-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Intl] Rename Regions to Countries #31350
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
Conversation
The region codes But otherwise, I agree about this renaming, as this class deals with a subset of ICU regions, not with all regions, and so is a different concept. |
You're right, i suggest to consistently ignore all "non" valid codes, as done in #28833 for languages
See also http://unicode.org/reports/tr35/#Unknown_or_Invalid_Identifiers (we almost conform) Do we agree excluding data is a new feature still? |
Thank you @ro0NL. |
This PR was merged into the 4.3-dev branch. Discussion ---------- [Intl] Rename Regions to Countries | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes (including intl-data group) | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Because that's what the current region data is about; country codes. This makes things consistent across the board; i.e. CountryType, CountryValidator This allows a possible other region subset (e.g "continents") to distinct. Thus having `Countries::class` + `Continents::class` both reading from the `region` data. By then data should be compiled under different keys. Current class is master only, so now or never :) The alternative approach would be `Regions::getCountryNames [,getContinentNames, etc.]`, which is harder to scale, and should also be decided for 4.3 ideally. Commits ------- 49aee67 [Intl] Rename Regions to Countries
I think we should revert this change. Regions are not the same as countries. There are lots of territorial disputes in the world which (in some cases) makes it impossible to define what a country is. Even ICU itself declares that:
Also, all the smart developers out there in all programming languages use "Region". You can also see that in all operating systems. Even when displaying it for end-users, they choose "region" over "country": |
@javiereguiluz we use the AFAIK the current data are true "countries", as we filter upstream ICU data. As such; did you find a case where a "2 letter region code" is NOT an expected country code? if so it should be patched IMHO (exclude those), see e.g. #31365 in general i think it's valid to provide a subset of regions; thus countries only. |
@ro0NL yes, I've found lots of errors :(
There are many other mistakes. The only reasonable way to fix all this is ... reverting this PR and using "Region" instead of "Country". That's what everybody else does. Please, consider a revert 🙏 Thanks! |
from a form perspective, not by Symfony: Is it a but then again, renaming everything to Region is also an option. But it's a bigger step for the codebase in terms of deprecation etc. And if we stick with "regions", does https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Intl/Data/Generator/RegionDataGenerator.php#L43 still apply? Technically we dont care anymore 🤔 |
i think we should filter by https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes which states
|
So following ISO-3316 seems like a good compromise? |
currently Symfony provides 255 country codes ( removing the aliases from https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes gives us 249 codes. I think we're very close already, and probably if we remove the remaining "islands" and "territories" we should be good. |
But then, it becomes our responsibility to filter regions, which is probably something we want to do in the long run. Woudn't it "better" to rename |
want or "dont want"? We already filter "some" regions today, but now im unsure what the intend is :) "exclude invalid regions" (what's an invalid region?) vs. "exclude invalid countries" (we know that's ISO-3166). If we dont filter "regions" at all the list effectively becomes https://github.com/unicode-org/icu/blob/master/icu4c/source/data/region/en.txt ... so from a domain perspective i value "countries" more than "regions", thus ISO-3166. |
Sorry, I means "don't want" here :) I do understand that we need to filter some entries that are not countries (World). But filtering countries is way more complex as Javier noted. I think it's not Symfony's responsibility to make choices here. So, sticking to ISO-3166 is an option, keeping regions is another one. I must admit that I don't have any preference here but I recognize that we need to make an informed decision here. |
I'd vote for sticking with ISO-3166 for now, thus "countries" and filter what's not an ISO-3166 code see https://www.diffchecker.com/OJeLGEYU it's doable. |
the process is also easy IMHO, whenever we apply an ICU update we should verify only ISO-3166 country codes are added |
@symfony/deciders We need your point of view here. Thank you. |
I'm for "Country" - the examples you gave Javier relate all to some geographical configuration. But if I want to create an "Address" field, I really want a "Country" label before entering "France". I think that's what this is for most of the time. |
Sticking to ISO-3166 seems fine and then it's technically countries. If someone wants to use a more generic term like "region" they can just change the label. But for most cases it's for adress fields as Nicolas said, and there only countries make sense because you cannot send a postcard to the EU or EZ. Or can you? Would be funny to try out. |
TLDR;
From https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2#Decoding_table
After #31365 SF will provide the "officially assigned ISO 3166-1 alpha-2 codes"; those we call "Countries", hence this PR. Thus, per ISO standards; e.g. Antarctica and Hong Honk are countries. Whether we can send a postcard to some address here is vague. For Hong Kong i believe yes, for Antarctica i believe not (yet there's a post office 😂 https://theplanetd.com/a-post-office-in-antarctica-you-betcha/) Im not aware of why Antarctica (a continent) is assigned an official country code, i guess it's some practical reason (as it's a continent without countries). |
…z script code (ro0NL) This PR was merged into the 4.3-dev branch. Discussion ---------- [Intl] Made countries ISO 3166 compliant + exclude Zzzz script code | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes (including intl-data group) | Fixed tickets | #31350 (comment), #18613 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> For consistency :) Commits ------- 1bb7dde [Intl] Made countries ISO 3166 compliant + exclude Zzzz script code
Actually, Antartica is split in pieces that rattached to existing countries: https://en.wikipedia.org/wiki/Antarctica#Antarctic_territories |
Because that's what the current region data is about; country codes.
This makes things consistent across the board; i.e. CountryType, CountryValidator
This allows a possible other region subset (e.g "continents") to distinct. Thus having
Countries::class
+Continents::class
both reading from theregion
data. By then data should be compiled under different keys.Current class is master only, so now or never :)
The alternative approach would be
Regions::getCountryNames [,getContinentNames, etc.]
, which is harder to scale, and should also be decided for 4.3 ideally.