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 improvements to translation pipeline #3555

Merged
merged 11 commits into from
May 27, 2021

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented May 26, 2021

This PR introduces the following improvements:

Cache Gutenberg translations in GB-mobile

The idea is to keep a controlled version of translations to workaround the potential issues of missing strings from Gutenberg. This will also help us to know exactly the translations we're bundling when cutting a release, currently, we're downloading them on demand when running the release script.

Regarding the issue WordPress/gutenberg#31932, we're going to keep the translations temporarily from the latest version (GB-mobile 1.52.2) that includes the missing strings. These files are included in this commit, once the issue is solved and the missing strings are restored we'll update the cache.

For updating the cache, we can run the command: cross-env REFRESH_I18N=1 npm run i18n:gutenberg. In fact, in the future, once we can rely on what's in Gutenberg's project on GlotPress, updating the cache should be part of the release script.

The cached files will overwrite the ones we have within Gutenberg in gutenberg/packages/react-native-editor/i18n-cache, which are actually the files that are bundled. I'd like to investigate further how to simplify this part but it's tricky because these files are also included in the demo project. Besides, once we include the translations from Jetpack this process will become even more complex 🤔 .

Fix commands for extracting the proper strings for the localization strings files

When a bundle is made, we generate the following localization strings files that are used in the main apps:

These files should only contain the strings that are exclusively used in native files, this means files with extensions native.js, ios.js, and android.js. If the string is also used in other files, typically used in the web version, it will be part of Gutenberg's project on GlotPress and therefore will be translated as part of it.

The rest will be included in the mentioned localization strings files and they will be merged with the rest of the strings that are in the main apps. Basically, all these strings get translated the same as Gutenberg but they're included in a different GlotPress project.

When the editor is initialized, we merge all the translations into a single object (related code).

Being this said, for identifying the only native strings we have to do the following steps:

  1. Extract non-native strings from Gutenberg: This is done by the command makepot:gutenberg that generates the file gutenberg.pot which includes all the strings in POT format.

  2. Extract native strings from Gutenberg: Similar to the previous step, we'll generate a POT file by running the commands makepot:android and makepot:ios but in this case, we'll have one file per platform (gutenberg-ios.pot and gutenberg-android.pot).
    It's important to note that we have an extra parameter (--subtract=./gutenberg.pot) in the command, this is used to subtract the strings that are shared between web and native.

  3. Convert POT files to native localization files: Each platform of the main apps require its own format, for this reason we have a couple of scripts that convert the POT files into the required formats (iOS: /bin/po2swift.js, Android: ./bin/po2android.js).

The makepot:* commands are using underneath the utility WP-CLI, specifically the i18n make-pot command. This command accepts different parameters but the important ones that are being used in our case are:

makepot:gutenberg

  • <source> => ./gutenberg/packages: We use the packages as the input as it includes all the code that have strings.

  • <destination> => gutenberg.pot: This file will be used for subtracting, as described below.

  • --ignore-domain: This allows us to extract all independently to the domain.

  • --exclude=test/*,e2e-tests/*,build/*,build-module/*,build-style/*,*.native.js,*.ios.js,*.android.js,bundle/*: We have to explicitly filter out all the files and folders related to native files or that we don't want to extract strings from. I'd like to mention that I had to add the build folders (build/* and build-module/*) because otherwise, the filtering is not working properly, and bundle folder to exclude the JS bundle generated on the demo project.

makepot:[ios|android]

  • <source> => ./gutenberg/packages: We use the packages as the input as it includes all the code that have strings.

  • <destination> => gutenberg-[ios|android].pot: The files that will be used for converting them to the native formats.

  • --ignore-domain: This allows us to extract all independently to the domain.

  • --subtract=./gutenberg.pot: This serves to filter out the strings shared with non-native files.

  • --include=*.native.js,*.[android|ios].js: We explicitly set the native file extensions.

  • --exclude=test/*,e2e-tests/*,build/*,build-module/*,build-style/*,*.js,*.php: As in makepot:gutengerg, we set the folders and files that we don't to extract strings from.
    As described in the make-pot command documentation, it wouldn't be quired to exclude .js or .php files due to the use of the include parameter, however, as far as I tested, this is not really working so we have to include them 🤷‍♂️ .

To test

Localization strings files shouldn't be modified

The current localization strings files in the develop branch have the proper strings so generating a new bundle shouldn't modify them.

Localization strings files:

  • bundle/android/strings.xml
  • bundle/ios/GutenbergNativeTranslations.swift

Steps:

  1. Run the command: npm run bundle
  2. Observe that localization strings files are not modified

Verify that translations are correct

We have to verify that translations are correct in the installable builds, for this purpose I created the following PRs in the main apps:

Steps:

Ideally, we should check different strings and verify that they have the proper translations, however, since they're a lot, we can focus on the block titles which actually originated this PR.

  1. Change the device's language to a non-English one (in my case I used Spanish)
  2. Open the app
  3. Open a post/page
  4. Tap on the ➕ button
  5. Observe that the block titles are not translated

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@fluiddot fluiddot self-assigned this May 26, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 26, 2021

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

"postmakepot:gutenberg": "npm run clean:gutenberg",
"pregenstrings": "test -f gutenberg.pot || npm run makepot:gutenberg",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The gutenberg.pot file can be different if new strings are introduced so I think it's not reliable to prevent generating it if it already exists.

@@ -61,7 +61,9 @@
"prern-bundle": "patch-package --patch-dir gutenberg/packages/react-native-editor/metro-patch",
"rn-bundle": "react-native bundle",
"postrn-bundle": "patch-package --reverse --patch-dir gutenberg/packages/react-native-editor/metro-patch",
"bundle": "npm run clean; npm run core prebundle && npm run bundle:js && npm run genstrings",
"i18n:gutenberg": "./bin/i18n-download.sh gutenberg",
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 future, once the missing strings issue is solved, we would change this command to cross-env REFRESH_I18N=1 npm run i18n:gutenberg. This way we would enforce the cache update when generating the bundle.

'zh-tw', // Chinese (Taiwan)
];

const getLanguageUrl = ( locale, plugin ) =>
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 file is almost the same as the one we have in gutenberg/packages/react-native-editor/i18n-cache but here I included the plugin parameter that will be useful for when we introduce the translations for Jetpack.

@@ -61,7 +61,9 @@
"prern-bundle": "patch-package --patch-dir gutenberg/packages/react-native-editor/metro-patch",
"rn-bundle": "react-native bundle",
"postrn-bundle": "patch-package --reverse --patch-dir gutenberg/packages/react-native-editor/metro-patch",
"bundle": "npm run clean; npm run core prebundle && npm run bundle:js && npm run genstrings",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to note that I removed npm run core prebundle from this command as now we're updating the i18n cache via the new i18n:gutenberg command.

@fluiddot fluiddot added this to the 1.54.0 (17.5) milestone May 27, 2021
Copy link

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @fluiddot, it will for sure make our release rotations a lot better🙇

I've started testing this by running the npm run bundle command and my localization strings files modified. I repeated the steps after an npm run clean in case that helped but the result was the same.

git status
On branch fix/translations-string-extraction
Your branch is up to date with 'origin/fix/translations-string-extraction'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   bundle/android/strings.xml
	modified:   bundle/ios/App.js
	modified:   bundle/ios/App.js.map
	modified:   bundle/ios/GutenbergNativeTranslations.swift

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	block-experiments/

no changes added to commit (use "git add" and/or "git commit -a")

git diff (⚠️ big attachment)

Probably something is different or wrong in my local setup 🤷

@fluiddot
Copy link
Contributor Author

Thank you for working on this @fluiddot, it will for sure make our release rotations a lot better🙇

I've started testing this by running the npm run bundle command and my localization strings files modified. I repeated the steps after an npm run clean in case that helped but the result was the same.

git status
git diff (⚠️ big attachment)

Probably something is different or wrong in my local setup 🤷

Oh interesting, looks like it's not adding all the strings that it should 🤔 . Now that you mention, I forgot to note that we need to use the latest version of WP-CLI (2.5.0) so probably the issue caused by that reason. I've just added a new command that will enforce WP-CLI to be up to date when it's used 🔧 .

@antonis
Copy link

antonis commented May 27, 2021

Thank you for the quick fix @fluiddot 👍
The localization string remained untouched 🎉
I'll proceed with the rest of the tests.

git status
On branch fix/translations-string-extraction
Your branch is up to date with 'origin/fix/translations-string-extraction'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   bundle/ios/App.js
	modified:   bundle/ios/App.js.map

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	block-experiments/

no changes added to commit (use "git add" and/or "git commit -a")

Copy link

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Great work @fluiddot 👍
I tested this in a few languages (Greek, Spanish, German) with the CI builds and the translations seem to be the expected. The code LGTM 🎉

@fluiddot
Copy link
Contributor Author

@antonis Heads up that I've updated the release notes, once the PR checks pass I'll merge it, thank you very much for the review ❤️ !

@fluiddot fluiddot merged commit f806eae into develop May 27, 2021
@fluiddot fluiddot deleted the fix/translations-string-extraction branch May 27, 2021 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants