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

Import missing translations #2305

Merged
merged 6 commits into from
Dec 1, 2021
Merged

Import missing translations #2305

merged 6 commits into from
Dec 1, 2021

Conversation

1pretz1
Copy link
Contributor

@1pretz1 1pretz1 commented Nov 30, 2021

These languages were originally missed out from the supplier (ASP) and so this PR imports them.

Original import PR: #2285

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 ⚠️

Import translations missed out of the original import from the supplier (ASP).

`bundle exec rake translation:import['dir/to/csv.csv']`
Renames the text keys and adds link.
bundle exec rake translation:normalize
@govuk-ci govuk-ci temporarily deployed to government-f-import-mis-rfdd59 November 30, 2021 17:48 Inactive
Copy link
Contributor

@ChrisBAshton ChrisBAshton left a comment

Choose a reason for hiding this comment

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

This is looking good - hardly any comments from me! Just a couple of questions below:

config/locales/gu.yml Show resolved Hide resolved
@@ -371,10 +371,10 @@ ur:
release_date: اجراء کی تاریخ
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed two keys whose values contain HTML, but the key names don't end in _html - were they missed?

Copy link
Contributor Author

@1pretz1 1pretz1 Dec 1, 2021

Choose a reason for hiding this comment

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

This confused me for a second! 😆 Those keys are actually unused and have been removed in the the following commit: 45d2065#diff-f8ecf4cd17cdd1ecc6f81401e44eba0a0e3c55ff9cd30b5acd1a3c4029fc5638L38-L39

Copy link
Contributor

Choose a reason for hiding this comment

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

Great - thanks for confirming!

@@ -631,6 +631,7 @@ ar:
engage_with_gov:
fcdo_bloggers:
follow:
follow_links:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know what the next steps are in terms of getting this key translated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, unfortunately! Currently there is no contingency plan for importing translations. The mission got cut short 😢 I'm hoping the next team that inherits this work (Find and view which I'm moving to) can prioritise it, but we'll see.

Safeguard against raw HTML being rendered in the application. See [1]
for more information.

[1]: https://guides.rubyonrails.org/i18n.html#using-safe-html-translations
bundle exec i18n-tasks remove-unused
bundle exec rake translation:add_missing
@1pretz1 1pretz1 force-pushed the import-missing-translations branch from 0e12395 to a2513fb Compare December 1, 2021 10:39
@govuk-ci govuk-ci temporarily deployed to government-f-import-mis-rfdd59 December 1, 2021 10:39 Inactive
@1pretz1 1pretz1 requested a review from ChrisBAshton December 1, 2021 10:50
Copy link
Contributor

@ChrisBAshton ChrisBAshton 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 answering my questions. ⭐

@1pretz1 1pretz1 merged commit f6f3115 into main Dec 1, 2021
@1pretz1 1pretz1 deleted the import-missing-translations branch December 1, 2021 10:54
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