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

Add translations and sync the locales #6369

Merged
merged 13 commits into from
Nov 25, 2021
Merged

Add translations and sync the locales #6369

merged 13 commits into from
Nov 25, 2021

Conversation

1pretz1
Copy link
Contributor

@1pretz1 1pretz1 commented Nov 19, 2021

Imports translations from the supplier (ASP). As part of this work, the translations have also been audited and synced.

More info in the commits ->

Trello:
https://trello.com/c/Xz78vNR4/2757-import-newly-sourced-translations-into-locale-files-5

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Imports translations directory from the outsourced key set provided by
ASP.
Now we've imported translations, this test fails as its expecting a key to be
blank, which is now present. Adds a stub to replicate the behaviour of a
blank key.
The space should've been before the colon, rather than after.
I believe this code was originally added to prevent documents for
certain languages (Arabic, Yiddish etc) assigning the documents slug the
same text as the title. Using text from these languages in the URL can
cause encoding errors.

The intention was to use the document id, instead of the title, for the
slug, if the `latin_script?` tag was false. However, this code is never
actually used and I don't think the issue is still present. I've tested
the behaviour in Arabic for a World News Article, and it successfully
publishes with the document id as the slug.
This was mistakenly capitalized
`bundle exec rake translation:normalize`
Adds keys `i18n-task remove-unused` will ignore, as they're generated
automatically.
`i18n-tasks remove-unused`
`bundle exec rake translation:add_missing`
Copy link
Contributor

@MuriloDalRi MuriloDalRi left a comment

Choose a reason for hiding this comment

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

This looks good, I'm just not sure why we need to remove unused keys, is that step necessary? Don't we want to keep those translations?

config/locales/fr.yml Outdated Show resolved Hide resolved
@1pretz1
Copy link
Contributor Author

1pretz1 commented Nov 23, 2021

Thanks for the review @MuriloDalRi!

I'm just not sure why we need to remove unused keys, is that step necessary? Don't we want to keep those translations?

It's not strictly necessary to remove unused keys as part of the import, but I thought it would be useful to remove them, rather than add keys that are not actually used in the application. If we were to add them then this would create more work for us when we come to validate the locales. It is sad that the locale files have changed since we sent the files off and quite a few of them are no longer needed.

@1pretz1 1pretz1 merged commit da2d262 into main Nov 25, 2021
@1pretz1 1pretz1 deleted the add-translations branch November 25, 2021 09:33
1pretz1 added a commit that referenced this pull request Nov 25, 2021
This was incorrectly changed when importing locales:
#6369
@1pretz1 1pretz1 mentioned this pull request Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants