feat(common): generate i18n data file `locale_all` file for closure by ocombe · Pull Request #18813 · angular/angular

ocombe · 2017-08-21T17:31:43Z

PR Type

What kind of change does this PR introduce?

[x] Feature

What is the current behavior?

Issue Number: #18763

What is the new behavior?

New all.ts file for closure, extracted using gulp cldr:locale-all.
You can choose the locales to use with the --locale parameter, e.g.: gulp cldr:locale-all --locales=fr,en,en-GB

Does this PR introduce a breaking change?

[x] No

Other information

This PR is a branch based on the PR #18284, don't merge before the other one is merged first.

mary-poppins · 2017-08-21T17:47:10Z

You can preview 766e77d at https://pr18813-766e77d.ngbuilds.io/.

vicb · 2017-08-22T21:31:22Z

You you please rebase the PR (ie remove the first 3 commits). Thanks

vicb · 2017-08-22T22:15:40Z

packages/common/i18n_data/locale_all.ts

remove this interface

vicb · 2017-08-22T22:15:50Z

packages/common/i18n_data/locale_all.ts

vicb · 2017-08-22T22:21:06Z

tools/gulp-tasks/cldr/extract.js

If having the code here make it required to download CLDR before generating the index please make it a distinct task so it could be invoked w/o downloading CLDR. Thanks

vicb · 2017-08-22T22:47:39Z

tools/gulp-tasks/cldr/extract.js

Please change to l only - good enough doe generated code.

mary-poppins · 2017-08-23T08:36:09Z

You can preview 4bc22c4 at https://pr18813-4bc22c4.ngbuilds.io/.

ocombe · 2017-08-23T08:54:43Z

New behavior: the script is now gulp cldr:locale-all instead of being bundled in the extract script.
If you run gulp cldr:locale-all you must run download first.
If you run gulp cldr:locale-all --locales=... you don't need to run download first, but the i18n locale data files must be available (they are versioned, so I don't see any reason why not, but it still checks just in case).

mary-poppins · 2017-08-23T09:33:27Z

You can preview cfb1c87 at https://pr18813-cfb1c87.ngbuilds.io/.

vicb · 2017-08-23T23:26:34Z

tools/gulp-tasks/cldr/locale-all.js

please change to import {registerLocaleData} from '../src/i18n/locale_data';

we need to merge #18857 first, because for now locale_data doesn't export registerLocaleData

vicb · 2017-08-23T23:26:46Z

tools/gulp-tasks/cldr/locale-all.js

please remove this line

I can't, bazel build (ci/circleci: build) fails if I do that...
@alexeagle any idea ?

No, what's the failure?

just that "goog" is undefined or something like that, which is logical since it's a global variable and it's not in any type definition for angular

ocombe · 2017-08-24T14:43:21Z

@vicb I did the changes that you requested but it fails the tests / build now because we need to merge #18857 first (locale_data doesn't export registerLocaleData for now), and since I removed the declaration for goog the bazel build fails like I said: https://circleci.com/gh/angular/angular/20113

mary-poppins · 2017-08-24T14:51:55Z

You can preview 8489458 at https://pr18813-8489458.ngbuilds.io/.

googlebot · 2017-08-28T18:09:17Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

Fixes angular#18763

mary-poppins · 2017-08-28T19:43:35Z

You can preview 7ae909f at https://pr18813-7ae909f.ngbuilds.io/.

vicb · 2017-08-28T20:04:50Z

Will be merged via #18907

angular-automatic-lock-bot · 2019-09-12T20:51:48Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

ocombe added area: i18n Issues related to localization and internationalization action: review The PR is still awaiting reviews from at least one requested reviewer state: blocked target: major This PR is targeted for the next major release labels Aug 21, 2017

ocombe requested a review from vicb August 21, 2017 17:31

googlebot added the cla: yes label Aug 21, 2017

ocombe changed the title ~~feat(common): generate i18n date file locale_all file for closure~~ feat(common): generate i18n data file locale_all file for closure Aug 21, 2017

ocombe force-pushed the google-locale-all branch from cc4fa07 to 766e77d Compare August 21, 2017 17:34

vicb approved these changes Aug 21, 2017

View reviewed changes

vicb removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Aug 21, 2017

vicb reviewed Aug 22, 2017

View reviewed changes

packages/common/i18n_data/locale_all.ts Outdated

Copy link
Copy Markdown

Contributor

vicb Aug 22, 2017

Choose a reason for hiding this comment

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

remove this interface

ocombe reacted with thumbs up emoji

vicb reviewed Aug 22, 2017

View reviewed changes

packages/common/i18n_data/locale_all.ts Outdated

Copy link
Copy Markdown

Contributor

vicb Aug 22, 2017

Choose a reason for hiding this comment

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

any

ocombe reacted with thumbs up emoji

vicb reviewed Aug 22, 2017

View reviewed changes

tools/gulp-tasks/cldr/extract.js Outdated

Copy link
Copy Markdown

Contributor

vicb Aug 22, 2017

Choose a reason for hiding this comment

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

Please change to l only - good enough doe generated code.

ocombe force-pushed the google-locale-all branch from 766e77d to 4bc22c4 Compare August 23, 2017 08:23

ocombe force-pushed the google-locale-all branch from 4bc22c4 to 0ed4c27 Compare August 23, 2017 08:50

ocombe force-pushed the google-locale-all branch from 0ed4c27 to cfb1c87 Compare August 23, 2017 09:08

angular deleted a comment from mary-poppins Aug 23, 2017

vicb reviewed Aug 23, 2017

View reviewed changes

vicb added 2 commits August 23, 2017 17:20

refactor(common): refactor i18n code to ease tree shaking

d691bb3

refactor(common): update generated i18n data

e9e8bfa

ocombe force-pushed the google-locale-all branch from cfb1c87 to 8489458 Compare August 24, 2017 14:38

ocombe force-pushed the google-locale-all branch from 8489458 to f7a6e52 Compare August 28, 2017 18:09

googlebot added cla: no and removed cla: yes labels Aug 28, 2017

ocombe force-pushed the google-locale-all branch from f7a6e52 to fe695d9 Compare August 28, 2017 19:12

ocombe added cla: yes and removed cla: no labels Aug 28, 2017

feat(common): generate i18n data file locale_all file for closure

7ae909f

Fixes angular#18763

ocombe force-pushed the google-locale-all branch from fe695d9 to 7ae909f Compare August 28, 2017 19:25

googlebot added cla: no and removed cla: yes labels Aug 28, 2017

vicb mentioned this pull request Aug 28, 2017

i18n code refactoring (intl removal) #18907

Closed

mhevery approved these changes Aug 28, 2017

View reviewed changes

mhevery added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release and removed target: major This PR is targeted for the next major release labels Aug 28, 2017

vicb closed this Aug 28, 2017

ocombe deleted the google-locale-all branch November 10, 2017 08:48

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019

Conversation

ocombe commented Aug 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

mary-poppins commented Aug 21, 2017

Uh oh!

vicb commented Aug 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mary-poppins commented Aug 23, 2017

Uh oh!

ocombe commented Aug 23, 2017

Uh oh!

mary-poppins commented Aug 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ocombe Aug 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ocombe Aug 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ocombe commented Aug 24, 2017

Uh oh!

mary-poppins commented Aug 24, 2017

Uh oh!

googlebot commented Aug 28, 2017

Uh oh!

mary-poppins commented Aug 28, 2017

Uh oh!

vicb commented Aug 28, 2017

Uh oh!

angular-automatic-lock-bot bot commented Sep 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ocombe commented Aug 21, 2017 •

edited

Loading

ocombe Aug 24, 2017 •

edited

Loading

ocombe Aug 24, 2017 •

edited

Loading