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

fix Norwegian and Dari hoc sync errors #31620

Merged
merged 2 commits into from Nov 5, 2019
Merged

Conversation

Hamms
Copy link
Contributor

@Hamms Hamms commented Oct 31, 2019

Description

Background

We've had a persistent issue with the HOC I18n sync wherein the YAML keys for Norwegian and Dari get malformed during the sync: 667045c

Both of these issues are a result of the fact that we were formerly using a regex to replace the top-level key provided by crowdin with one of our own:

# replace the crowdin code in the file itself with our own unique
# language code
File.write(new_path, File.read(new_path).gsub(/'#{prop[:crowdin_code_s]}':/, "#{prop[:unique_language_s]}:"))

In the case of Norwegian, the problem was that the key no: is actually parsed by YAML as a boolean and not as a string, so we should be escaping that code like 'no':, but the regex doesn't know that.

In the case of Dari, the problem is that the top-level key provided by crowdin is actually not the unique code (which for dari is fa-AF) but is actually just the two-letter language code, which in the case of Dari is fa and which overlaps with the two-letter code for Persian.

Fix

In both cases, the best fix is to actually parse the YAML, make our changes to the resulting data object, and then write the result back out to YAML.

Note that this will have a side effect of introducing formatting changes to the files. These files are YAML and there's no such thing as a consistent YAML format, so this is to be expected. Changes are mostly of the form of adding or removing quotes from strings. For example:

diff --git a/pegasus/sites.v3/hourofcode.com/i18n/no.yml b/pegasus/sites.v3/hourofcode.com/i18n/no.yml
index 41eb5daacd3..501a3819ca5 100644
--- a/pegasus/sites.v3/hourofcode.com/i18n/no.yml
+++ b/pegasus/sites.v3/hourofcode.com/i18n/no.yml
@@ -1,60 +1,61 @@
+---
 'no':
-  site_name: 'Kodetimen'
-  hour_of_code: 'Kodetimen'
-  language: 'Norsk Bokmål'
-  front_title: 'Bli med på historiens største læringsarrangement, %{campaign_date}'
-  coming_soon: 'Kommer snart'
-  campaign_date_start_short: "4. desember"
-  campaign_date_start_long: "4. desember"
-  campaign_date_short: "4.-10. Desember"
-  campaign_date_full: "4.-10. desember"
-  campaign_date_year: "2017"
-  campaign_date_full_year: "4.-10. desember 2017"
-  hoc2019_header: 'Lær om teknologi for å forandre verden.'
-  hoc2019_header_line1_mobile: 'Lær om teknologi.'
-  hoc2019_header_line2_mobile: 'Forandre verden.'
+  site_name: Kodetimen
+  hour_of_code: Kodetimen
+  language: Norsk Bokmål
+  front_title: Bli med på historiens største læringsarrangement, %{campaign_date}
+  coming_soon: Kommer snart
+  campaign_date_start_short: 4. desember
+  campaign_date_start_long: 4. desember
+  campaign_date_short: 4.-10. Desember
+  campaign_date_full: 4.-10. desember
+  campaign_date_year: '2017'
+  campaign_date_full_year: 4.-10. desember 2017
+  hoc2019_header: Lær om teknologi for å forandre verden.
+  hoc2019_header_line1_mobile: Lær om teknologi.
+  hoc2019_header_line2_mobile: Forandre verden.
   social_hoc2019_every_sudent: 'Every student has the potential to change the world. Help them get started. #CSforGood'
   social_hoc2019_learn_computer_science: 'Learn computer science. Change the world. #CSforGood #HourOfCode'
-  social_hoc_is_coming: 'The Hour of Code is coming!'
+  social_hoc_is_coming: The Hour of Code is coming!
   social_hoc2019_hoc_is_about_csforgood: 'The #HourOfCode is about #CSforGood. Learn computer science. Change the world.'

Links

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@Hamms Hamms requested a review from a team October 31, 2019 20:41
@daynew daynew self-requested a review November 5, 2019 19:49
Copy link
Member

@daynew daynew left a comment

Choose a reason for hiding this comment

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

Looks like it is the same behavior to me and it's pretty easy to

@Hamms
Copy link
Contributor Author

Hamms commented Nov 5, 2019

FYI @joshlory that with this PR we expect to see some formatting changes next sync

@Hamms Hamms merged commit cf80065 into staging Nov 5, 2019
@Hamms Hamms deleted the fix-no-and-fa-hoc-sync-errors branch November 5, 2019 20:36
Hamms added a commit that referenced this pull request Nov 7, 2019
Followup to #31620

Prior to the linked PR, we would get the downloaded translation data into the right place by renaming the downloaded files and then making manual edits to them.

The linked PR made things a little more precise by having us instead actually parse the data from the downloaded files, modify that data, and then write the results out to the new location.

Unfortunately, this means we ended up with files sticking around at the downloaded location. I forgot to add a `rm`.
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.

None yet

2 participants