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

Modify sync-in to also gather function descriptions #34568

Merged
merged 1 commit into from May 6, 2020

Conversation

bethanyaconnor
Copy link
Contributor

First PR in the process of translating function descriptions.

Process I'm following:

  1. (This PR) Update the sync-in script to gather the function descriptions. I'm changing the structure very slightly so that it's structured like:
level_url: {
  function_definitions: {
    function_name: {
      name: "function_name"
      description: "function description"
    }
  }
 }

Note that the key changed from function_names to function_definitions. I did this to make the transition a bit easier, plus it makes more sense.
2. Before running a sync that contains this PR, upload a file with the existing function names. This will allow duplication to work correctly.
3. Run a sync with this change. The out step will produce function_defintions.*.yml and will not update function_names.*.yml. Check in the function_defintions.*.yml files.
4. Modify localized_function_blocks to read from the new translation files. As part of that PR, delete function_names.*.yml.

This process limits the management of PR/sync timing to step 2. The sync out needs to happened before step 4, but there isn't any complicated PR juggling (i.e. making sure the updated rendering logic the sync go out in the same DTP).

Links

Testing story

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

@bethanyaconnor bethanyaconnor requested a review from a team as a code owner May 1, 2020 20:34
description = function.at_xpath('./mutation/description')
i18n_strings['function_defintions'][name.content] = Hash.new
i18n_strings['function_defintions'][name.content]["name"] = name.content if name
i18n_strings['function_defintions'][name.content]["description"] = description.content if description
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible for there to be functions defined in two places that share the same name but have different descriptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the same level? That seems unlikely but I haven't checked

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, right! I forgot we broke these up by level

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.

Both code and process look good to me! I love having the process laid out here so explicitly, too!

@bethanyaconnor
Copy link
Contributor Author

Uploaded a JSON with the function names to Crowdin -- all duplicates as expected. I'm going to merge this PR

@bethanyaconnor bethanyaconnor merged commit 93b9945 into staging May 6, 2020
@bethanyaconnor bethanyaconnor deleted the translate-artist-function-descriptions branch May 6, 2020 21:16
@bethanyaconnor bethanyaconnor mentioned this pull request May 19, 2020
7 tasks
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