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 for "AI for Oceans" activity #32362

Merged
merged 4 commits into from Jan 6, 2020
Merged

i18n for "AI for Oceans" activity #32362

merged 4 commits into from Jan 6, 2020

Conversation

bethanyaconnor
Copy link
Contributor

@bethanyaconnor bethanyaconnor commented Dec 9, 2019

Description

Relies on a merge and publish of code-dot-org/ml-activities#343.

This approach is based off how we do i18n in Blockly. In ml-activities, we will expose a json of the strings we use. Our i18n pipeline will copy that file into the source directory and sync it to Crowdin.

Because we currently don't have any other fish strings, I'm copying the ml-activities source strings to i18n/locales/source/blockly-mooc/fish.json which will mean (after a full sync), translations will end up in apps/i18n/fish/<lang>.json which should allow translations to "just work". However, if we ever add strings to this activity directly in this repo, we'll need to change this approach (it will overwrite any string).

Open to thoughts/suggestions on this approach.

Full process for translating ml-activities strings

In the code-dot-org repo, we use MessageFormat in our apps code. This repo will use the same version. The steps the strings will go through to be translated are:

  1. Strings are added to oceans.json
  2. The i18n pipeline will copy this file into a source directory and upload it to Crowdin
  3. The strings get translated on Crowdin
  4. The translations are downloaded and distributed to apps/i18n/fish/<lang>.json
  5. The translations are passed into the initialization of the fish tutorial.

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
Copy link
Contributor

Hamms commented Dec 9, 2019

I am concerned about step 5; that process is currently undefined for our Blockly I18n, and I'd really like to make sure we have a solid plan for how to address that (for both this and Blockly, ideally) before we commit to this pattern.

@bethanyaconnor
Copy link
Contributor Author

Yeah, I don't love the pattern we have for Blockly and I wasn't super excited to commit harder to it. Open to thoughts on better ways to handle this.

However, I don't fully understand what your question is. What is undefined?

@bethanyaconnor bethanyaconnor changed the base branch from staging-next to staging December 18, 2019 17:27
@bethanyaconnor
Copy link
Contributor Author

@Hamms after our discussion last week, any feedback on this PR? I think this goes in the general direction we talked about on Friday

Copy link
Contributor

@Hamms Hamms 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 great! I'm excited to see how this approach works in practice and to update our blockly i18n strategy to match

@bethanyaconnor bethanyaconnor marked this pull request as ready for review January 6, 2020 21:36
@bethanyaconnor bethanyaconnor requested a review from a team as a code owner January 6, 2020 21:36
@bethanyaconnor
Copy link
Contributor Author

I'm bumping ml-activities here as if we do need to rollback the ml-activities bump, we'll also need to rollback the i18n pipeline change.

@bethanyaconnor bethanyaconnor merged commit 774a8ba into staging Jan 6, 2020
@bethanyaconnor bethanyaconnor deleted the oceans-i18n branch January 6, 2020 23:18
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

3 participants