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

Jump to bottom

i18n: support xmb formatted translation files in compile time inlining #33444

Closed
wants to merge 2 commits into from
Closed

i18n: support xmb formatted translation files in compile time inlining #33444

wants to merge 2 commits into from

Conversation

petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented Oct 28, 2019 •
edited

No description provided.

@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer area: i18n target: major This PR is targeted for the next major release comp: ivy labels Oct 28, 2019
@petebacondarwin petebacondarwin requested review from a team as code owners October 28, 2019 14:45
@ngbot ngbot bot modified the milestone: needsTriage Oct 28, 2019
AndrewKushnir
AndrewKushnir approved these changes Oct 28, 2019
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for additional refactoring as well 👍

Pete, I'm wondering if we can add an e2e test that would:

  • generate XMB file based on an app (using ng xi18n CLI or APIs)
  • update XMB file (simulating translation changes)
  • run i18n inlining process (that would also involve XMB parsing)
  • verify that the page contains what we expect
    That might be helpful to make sure that extraction and inlining (consuming part) are in sync.
    Thank you.

@AndrewKushnir
Copy link
Contributor

@petebacondarwin this PR seems to have merge conflicts now, since the base PR is now merged. Could you plz rebase and resolve conflicts? Thank you.

@AndrewKushnir AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 28, 2019
@petebacondarwin
refactor(ivy): i18n - share MessageSerializer across `TranslationPa…
0e03cc4 
…rsers`

Each of the XML based `TranslationParsers` was providing its own
`MessageSerializer`, but they are all very similar. So these have been
consolidated into a single more generic `MessageSerializer.

As a result of this, the extra layers of folders in the project seemed
unnecessary, so they have been flattened.
@petebacondarwin
feat(ivy): i18n - support inlining of XTB formatted translation files
669ab79 
This commit implements the `XtbTranslationParser`, which can read XTB
formatted files.
@petebacondarwin
Copy link
Member Author

@AndrewKushnir - there is a test here that does exactly what you suggest - except that it uses the XLIFF 1.2 format rather than XMB. See https://github.com/angular/angular/blob/master/integration/cli-hello-world-ivy-i18n/package.json#L23

Was your suggestion that we should duplicate this test for XMB?

@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 28, 2019
@petebacondarwin petebacondarwin removed the request for review from a team October 28, 2019 20:16
@AndrewKushnir
Copy link
Contributor

Was your suggestion that we should duplicate this test for XMB?

@petebacondarwin yeah, I was wondering if we can extend existing test to cover other formats as well.

AndrewKushnir pushed a commit that referenced this pull request Oct 28, 2019
@petebacondarwin @AndrewKushnir
feat(ivy): i18n - support inlining of XTB formatted translation files (…
2c623fd 
…#33444)

This commit implements the `XtbTranslationParser`, which can read XTB
formatted files.

PR Close #33444
@petebacondarwin petebacondarwin deleted the i18n-parse-xmb branch October 29, 2019 08:33
petebacondarwin added a commit to petebacondarwin/angular that referenced this pull request Oct 29, 2019
@petebacondarwin
test(ivy): i18n - add XMB e2e integration test
58fb152 
This integration test now does a full e2e test of:

* extraction -> build -> translation - serve

for both XLIFF 1.2 and XMB formats.

Resolves angular#33444 (comment)
@petebacondarwin petebacondarwin mentioned this pull request Oct 29, 2019
Closed
AndrewKushnir pushed a commit that referenced this pull request Oct 29, 2019
@petebacondarwin @AndrewKushnir
test(ivy): i18n - add XMB e2e integration test (#33462)
f4160b5 
This integration test now does a full e2e test of:

* extraction -> build -> translation - serve

for both XLIFF 1.2 and XMB formats.

Resolves #33444 (comment)

PR Close #33462
matsko pushed a commit to matsko/angular that referenced this pull request Oct 30, 2019
@petebacondarwin @matsko
test(ivy): i18n - add XMB e2e integration test (angular#33462)
f95e636 
This integration test now does a full e2e test of:

* extraction -> build -> translation - serve

for both XLIFF 1.2 and XMB formats.

Resolves angular#33444 (comment)

PR Close angular#33462
mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019
@petebacondarwin @mohaxspb
refactor(ivy): i18n - share MessageSerializer across `TranslationPa…
9d62024 
…rsers` (angular#33444)

Each of the XML based `TranslationParsers` was providing its own
`MessageSerializer`, but they are all very similar. So these have been
consolidated into a single more generic `MessageSerializer.

As a result of this, the extra layers of folders in the project seemed
unnecessary, so they have been flattened.

PR Close angular#33444
mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019
@petebacondarwin @mohaxspb
feat(ivy): i18n - support inlining of XTB formatted translation files (…
7cc435e 
…angular#33444)

This commit implements the `XtbTranslationParser`, which can read XTB
formatted files.

PR Close angular#33444
mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019
@petebacondarwin @mohaxspb
test(ivy): i18n - add XMB e2e integration test (angular#33462)
ce13ed1 
This integration test now does a full e2e test of:

* extraction -> build -> translation - serve

for both XLIFF 1.2 and XMB formats.

Resolves angular#33444 (comment)

PR Close angular#33462
mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019
@petebacondarwin @mohaxspb
refactor(ivy): i18n - share MessageSerializer across `TranslationPa…
755689e 
…rsers` (angular#33444)

Each of the XML based `TranslationParsers` was providing its own
`MessageSerializer`, but they are all very similar. So these have been
consolidated into a single more generic `MessageSerializer.

As a result of this, the extra layers of folders in the project seemed
unnecessary, so they have been flattened.

PR Close angular#33444
mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019
@petebacondarwin @mohaxspb
feat(ivy): i18n - support inlining of XTB formatted translation files (…
b8991ba 
…angular#33444)

This commit implements the `XtbTranslationParser`, which can read XTB
formatted files.

PR Close angular#33444
mohaxspb pushed a commit to mohaxspb/angular that referenced this pull request Nov 7, 2019
@petebacondarwin @mohaxspb
test(ivy): i18n - add XMB e2e integration test (angular#33462)
08c0b38 
This integration test now does a full e2e test of:

* extraction -> build -> translation - serve

for both XLIFF 1.2 and XMB formats.

Resolves angular#33444 (comment)

PR Close angular#33462
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: i18n cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Milestone
needsTriage
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@petebacondarwin @AndrewKushnir @googlebot