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

[Intl] Revise timezone name generation #31434

Merged
merged 1 commit into from May 9, 2019
Merged

[Intl] Revise timezone name generation #31434

merged 1 commit into from May 9, 2019

Conversation

ro0NL
Copy link
Contributor

@ro0NL ro0NL commented May 8, 2019

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes (inlcluding intl-data group)
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

This is the final polishing needed for #31294 :)

I've realized it's much easier to de-duplicate by processing fallback locales separate, and then only keep the diff compared to a specific locale. More or less the same approach LocaleDataGenerator already follows. I was trying to be clever and filter based on inheritance in a single process; bad idea.

Includes https://github.com/ro0NL/symfony/commit/31591d0 (ref #31432)

@@ -151,7 +151,7 @@
"America\/Moncton": "Atlantic Time (Moncton)",
"America\/Monterrey": "Central Time (Monterrey)",
"America\/Montevideo": "Uruguay Time (Montevideo)",
"America\/Montreal": "Canada (Montreal)",
"America\/Montreal": "Canada Time (Montreal)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

use the regionFormat for country-based names, this is much more consistent with explicitly translated zones, e.g. Coordinated Universal Time and Central European Time

@@ -99,7 +99,6 @@
"Indian\/Mayotte": "heure d’Afrique orientale (Mayotte)",
"Indian\/Reunion": "heure de la Réunion",
"PST8PDT": "heure du Pacifique",
"Pacific\/Easter": "heure de l’île de Pâques",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's the bugfix; this should be inherited from fr locale

@@ -0,0 +1,8 @@
{
"Version": "2.1.49.82",
"Meta": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

root metadata is now compiled into root.json, in order to de-duplicte locale metadata

@@ -3,20 +3,8 @@
"Names": {
"Africa\/Kigali": "U Rwanda (Kigali)",
"Antarctica\/Troll": "Troll",
"CST6CDT": "CST6CDT",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should never provide zone IDs as translations, in this case the runtime fallback should apply instead.

{
"Version": "2.1.47.86",
"Names": {
"Africa\/Abidjan": "格林尼治标准时间(阿比贊)",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only alias locales are deleted (related to #31411)

@fabpot
Copy link
Member

fabpot commented May 8, 2019

Can you rebase to get green tests?

@nicolas-grekas nicolas-grekas changed the base branch from master to 4.3 May 9, 2019 07:50
@nicolas-grekas nicolas-grekas added this to the 4.3 milestone May 9, 2019
@nicolas-grekas
Copy link
Member

Ready for rebase on 4.3.

@ro0NL
Copy link
Contributor Author

ro0NL commented May 9, 2019

Done. (failures unrelated)

@nicolas-grekas
Copy link
Member

Thank you @ro0NL.

@nicolas-grekas nicolas-grekas merged commit bfdb4ed into symfony:4.3 May 9, 2019
nicolas-grekas added a commit that referenced this pull request May 9, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

 [Intl] Revise timezone name generation

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes (inlcluding intl-data group)
| Fixed tickets | #...   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

This is the final polishing needed for #31294 :)

I've realized it's much easier to de-duplicate by processing fallback locales separate, and then only keep the diff compared to a specific locale. More or less the same approach `LocaleDataGenerator` already follows. I was trying to be clever and filter based on inheritance in a single process; bad idea.

Includes https://github.com/ro0NL/symfony/commit/31591d0 (ref #31432)

Commits
-------

bfdb4ed [Intl] Revise timezone name generation
@ro0NL ro0NL deleted the tz-names branch May 9, 2019 14:08
fabpot added a commit that referenced this pull request May 13, 2019
…mezoneType (ro0NL)

This PR was merged into the 4.3 branch.

Discussion
----------

[Form] Add intl/choice_translation_locale option to TimezoneType

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #28836
| License       | MIT
| Doc PR        | symfony/symfony-docs#11503

final step :)

for now i think any form of grouping is a user concern (i.e. by GMT offset or area name); see #31293 + #31295

having a special built in `group_by' => 'gmt_offset'` util would be nice, and can be done in the future.

includes #31434

Commits
-------

001b930 [Form] Add intl/choice_translation_locale option to TimezoneType
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.

None yet

4 participants