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 new locale #1382

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Add new locale #1382

merged 1 commit into from
Jun 24, 2019

Conversation

MuriloDalRi
Copy link
Contributor

Adds support for Kazakh (kk)

Trello


Visual regression results:
https://government-frontend-pr-[THIS PR NUMBER].surge.sh/gallery.html

Component guide for this PR:
https://government-frontend-pr-[THIS PR NUMBER].herokuapp.com/component-guide

@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 19, 2019 15:07 Inactive
Copy link
Contributor

@bevanloon bevanloon left a comment

Choose a reason for hiding this comment

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

I think we also want to add the locale file to config/locales as kk.yml. There are a number of rake tasks that can help with auto-generating the locales:

> rake -T | grep translation
rake translation:export[directory,base_locale,target_locale]      # Export a specific locale to CSV
rake translation:export:all[directory]                            # Export all locales to CSV files
rake translation:import[locale,path]                              # Import a specific locale CSV to YAML within the app
rake translation:import:all[directory]                            # Import all locale CSV files to YAML within the app
rake translation:regenerate[directory]                            # Regenerate all locales from the EN locale - run this after adding keys
rake translation:steal[locale,source_app_path,mapping_file_path]  # Import and convert a locale file from another app
rake translation:steal:all[source_app_path,mapping_file_path]     # Import and convert all locale files from another app
rake translation:validate                                         # Check translation files for errors

which I think come from rails_translation_manager.

You'll probably also want to include kk: Kazakh in config/locales/en.yml much in the same way as you have in https://github.com/alphagov/whitehall/pull/4861/files#diff-37c9b835927e3c363588f172de9bb6faR65

@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 20, 2019 09:55 Inactive
@MuriloDalRi
Copy link
Contributor Author

@bevanloon I regenerated the translations and imported all as YML.

The docs say There's no timeline for how frequently this is done, so you can expect many translation values to be missing in non EN locales.

So I'm assuming that's what happened here and that's ok? Or should I import just the kk one?

@bevanloon
Copy link
Contributor

bevanloon commented Jun 20, 2019

@MuriloDalRi yeah -I think I just added what I needed last time rather than all the extras, but it's a judgement call really. This deletion, however, doesn't look quite right to me and might account for your test failure (there might be others...I haven't scanned all the files).

Perhaps the translation manager isn't smart enough to deal with that.

@MuriloDalRi
Copy link
Contributor Author

MuriloDalRi commented Jun 20, 2019

@bevanloon Okay I'll have a look at fixing that but if it's too much faff I'll just add the Kazakh one. Just adding the kk one... I found this old issue which seems like it might be the same problem: #254

@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 20, 2019 10:45 Inactive
@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 20, 2019 10:48 Inactive
@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 20, 2019 10:48 Inactive
@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 20, 2019 10:52 Inactive
Copy link
Contributor

@bevanloon bevanloon left a comment

Choose a reason for hiding this comment

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

👍 looks right to me

@elliotcm elliotcm temporarily deployed to government-frontend-pr-1382 June 20, 2019 14:58 Inactive
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.

4 participants