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

docs(aio): Update i18n example to Angular V6 #23660

Closed

Conversation

brandonroberts
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@mary-poppins
Copy link

You can preview ebc15a0 at https://pr23660-ebc15a0.ngbuilds.io/.

@mary-poppins
Copy link

You can preview bd8fbdc at https://pr23660-bd8fbdc.ngbuilds.io/.

@jenniferfell jenniferfell added this to the v6-candidates milestone May 3, 2018
@ocombe
Copy link
Contributor

ocombe commented May 3, 2018

hmm is it really possible to mix configurations?
--prod will use configuration production and --configuration=fr will use configuration fr, so ng build --prod --configuration=fr will mix them up? If that's the case it's awesome

@ocombe
Copy link
Contributor

ocombe commented May 3, 2018

Confirmed by the CLI team, ng build --prod --configuration=fr won't work :(

Copy link
Contributor

@ocombe ocombe left a comment

Choose a reason for hiding this comment

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

Using a configuration seems really nice in theory, but you cannot use --prod with --configuration because --prod is an alias for --configuration=production, and the configurations are not additive.
We should either stick with the cli parameters, or add a "production:fr" configuration that includes everything from "production".

"build": "ng build --prod",
"build:fr": "ng build --prod --i18nFile=src/locale/messages.fr.xlf --i18nFormat=xlf --locale=fr",
"build:fr": "ng build --prod --configuration=fr",
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not work unfortunately
you either need to specify all i18n parameters and use --prod, or create a new prod fr config that includes everything from the production environment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it to include a production-fr configuration for production builds

"core-js": "^2.4.1",
"rxjs": "^5.5.2",
"zone.js": "^0.8.14"
"@angular/animations": "^6.0.0-rc.5",
Copy link
Contributor

@ocombe ocombe May 3, 2018

Choose a reason for hiding this comment

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

you can use rc6, or maybe just use 6.0.0 that will be released very soon?

@mary-poppins
Copy link

You can preview b04f31a at https://pr23660-b04f31a.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 1b1f661 at https://pr23660-1b1f661.ngbuilds.io/.

Copy link
Contributor

@ocombe ocombe left a comment

Choose a reason for hiding this comment

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

You can update the dependencies to 6.0.0 since it has just been released!

@brandonroberts
Copy link
Contributor Author

@ocombe made fixes and upgraded dependencies to V6.

@mary-poppins
Copy link

You can preview e12f469 at https://pr23660-e12f469.ngbuilds.io/.

@sandangel sandangel mentioned this pull request May 5, 2018
20 tasks
@ocombe ocombe added target: patch This PR is targeted for the next patch release area: i18n labels May 7, 2018
}
</code-example>

You then pass the configuration with the `ng serve` or `ng build` commands. The example below shows how to serve the French language file created in previous sections of this guide:
Copy link
Member

Choose a reason for hiding this comment

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

long line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


For production builds, you define a separate `production-fr` build configuration in your `angular.json`.

<code-example format="." language="ts">
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what format="." does but I am pretty sure it is not needed any more.

Copy link
Member

Choose a reason for hiding this comment

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

In fact if you are just putting inline code then you should probably just use triple backtickets.
<code-example> is really only for embedding examples written externally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Member

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

The change that moves all the examples to 6.0.0 looks scary :-)
But I guess if CI is green then we are good.

A couple of long lines that could be shortened.

And not the comment about inline code examples.

@petebacondarwin petebacondarwin 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 May 7, 2018
@ocombe ocombe 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 labels May 7, 2018
@mary-poppins
Copy link

You can preview 4e72d3c at https://pr23660-4e72d3c.ngbuilds.io/.

@ocombe ocombe removed the request for review from IgorMinar May 7, 2018 13:57
IgorMinar pushed a commit that referenced this pull request May 8, 2018
@IgorMinar IgorMinar closed this in 9e2d87f May 8, 2018
@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 Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: i18n cla: yes target: patch This PR is targeted for the next patch release
Projects
No open projects
docs
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

7 participants