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

I18n sync Down & Out 07/30 #23991

Merged
merged 78 commits into from Jul 31, 2018
Merged

Conversation

Hamms
Copy link
Contributor

@Hamms Hamms commented Jul 30, 2018

Regular sync out, but now with Crowdin's "skip untranslated strings" option enabled!

Depends on #23948

Copy link
Contributor

@tanyaparker tanyaparker left a comment

Choose a reason for hiding this comment

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

I have some questions, but if the things I commented out are all expected then it's probably good to go if you tested everything. Blank/missing strings makes me nervous!

sample4_failure_message_override: Congratulations! You gathered X coins!
sample6_failure_message_override: Not quite. Keep trying! [Move to Play Zone]
sample8_failure_message_override: Congratulations! You gathered X coins!
el:
Copy link
Contributor

Choose a reason for hiding this comment

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

This file went blank. Guessing this is fine since we didn't lose any translations and we fallback to the source en file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! Ram did a bunch of work last week to verify that fallbacks should always work

@@ -10,26 +10,26 @@
"COLLAPSE_BLOCK": "Bloku yığ",
"COLOUR_BLEND_COLOUR1": "rəng 1",
"COLOUR_BLEND_COLOUR2": "rəng 2",
"COLOUR_BLEND_HELPURL": "http://meyerweb.com/eric/tools/color-blend/",
"COLOUR_BLEND_HELPURL": "",
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we lose all these links because of sanitization?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or just so happens these are links but it's because they're not translated they're skipped like the rest? Should these even be in the i18n pipeline since they're only URLs and no string content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, these are skipped only because they're not translated. It probably doesn't make sense for them to be in the pipeline in the first place, but at least now they're not cluttering up the locales directory!

@@ -56,7 +56,7 @@
"pathRight": "əgər sağa yol varsa,",
"pilePresent": "təpəcik var",
"playSoundBounce": "sıçrayış səsi çıxart",
"playSoundCheer": "play cheering sound",
"playSoundCheer": "",
Copy link
Contributor

Choose a reason for hiding this comment

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

How come some files we'll show a blank string? But others we'll remove the key too? Or is the Greek file an exception/bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just the way crowdin does it; it looks like yml files remove the key and json files just drop it down to an empty string.

I'm eventually going to look into adding a step to our out sync to remove these, but they're harmless.

@Hamms
Copy link
Contributor Author

Hamms commented Jul 30, 2018

Yep, everything here has been well-tested. And of course, all the blank/missing strings are just untranslated English strings, so although it's a big negative diff we're actually losing literally zero data.

@Hamms Hamms merged commit 9694044 into i18n-sync-in-up-07-30 Jul 31, 2018
@Hamms Hamms deleted the i18n-sync-down-out-07-30 branch July 31, 2018 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants