-
Notifications
You must be signed in to change notification settings - Fork 215
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
Enable i18n with fr(French) for website #601
Conversation
✅ Deploy Preview for tag-app-delivery ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
e385bfc
to
23d5f45
Compare
Ready for your review when you have a minute @abangser and @antoine-sncf, thanks! |
Actually back to draft, as I need to remove the symlinks like in there #599. Still working on this. |
Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Chris Abraham <[email protected]> Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Mathieu Benoit <[email protected]>
I think this is now ready for your review @abangser, @antoine-sncf and @guillermotti. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good except that you may have pulled in one commit off main during a rebase. Specifically Try "exit 1" in both netlify.toml files to force all builds
.
I think it may be ok (hence the approve) but it may make history look a bit funny!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, need to block after learning about the URL field causing issues on the Spanish translation!
I think it's a small fix, but want to try and learn from that.
The test will be if opening up the maturity model shows as a default language of English or not in the language drop down.
…model and wgs/artifacts/charter/charter.md/ Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Mathieu Benoit <[email protected]>
Signed-off-by: Mathieu Benoit <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix @mathieu-benoit! Can confirm that the languages do work correctly, though as you can see in #608 I did spot an unassociated bug which may require a rebase depending on when this gets added.
Very happy for this to be merged now though as it has some french native reviews and appears to work on testing 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Like done with
es
here: #594, this PR is enabling the website infr
.Here are the 2 pages translated in French to review at this stage:
website/config.toml
website/i18n/fr.toml
The other files are a copy of
website/content/en
intowebsite/content/fr
as-is, without any French translation for now (this will be done later in future associated PRs). What was also done is removing these 4 symlinks and the associated content was copied manually:website/content/fr/wgs/platforms/glossary/
website/content/fr/wgs/platforms/maturity-model/
website/content/fr/wgs/platforms/whitepaper/
website/content/fr/wgs/operator/whitepaper
Also, these following are taken into account:
When this PR will be ready, the goal will be to have the reviews from:
es
translation