woocommerce-android
woocommerce-android copied to clipboard
Update automation that freezes strings for translation
Description
After the prep work from #6741 and #6703, this PR updates the automation to freeze the strings for translation.
Notably it:
- Uses the new
source_idparameter to differentiate the generated strings - Uses the Fastlane APIs to interact with Git
- Uses a structure consistent with WordPress Android
Testing instructions
To test this, I created a new branch from this one, hacked the complete_code_freeze lane to work outside of a release branch, and used it to verify the process succeeds. You can see it, including the commit with the new frozen strings, in #6800, but for reference here's the lane output
β bf complete_code_freeze
[β] π
[14:46:14]: ------------------------------
[14:46:14]: --- Step: default_platform ---
[14:46:14]: ------------------------------
[14:46:14]: Driving the lane 'android complete_code_freeze' π
[14:46:14]: -------------------
[14:46:14]: --- Step: is_ci ---
[14:46:14]: -------------------
[14:46:14]: ---------------------------------------
[14:46:14]: --- Step: check_for_toolkit_updates ---
[14:46:14]: ---------------------------------------
[14:46:14]: Currently using release toolkit version 4.2.0.
[14:46:14]: Checking for updates now...
[14:46:16]: Your release toolkit is up-to-date! β
[14:46:16]: -------------------------------------------------------
[14:46:16]: --- Step: Switch to android localize_libraries lane ---
[14:46:16]: -------------------------------------------------------
[14:46:16]: Cruising over to lane 'android localize_libraries' π
[14:46:16]: ----------------------------------------------
[14:46:16]: --- Step: android_download_file_by_version ---
[14:46:16]: ----------------------------------------------
[14:46:16]: Downloading WordPressLoginFlow/src/main/res/values/strings.xml from wordpress-mobile/WordPress-Login-Flow-Android at version 0.13.0 to /var/folders/29/wn6913pn73gf2l7j5n4n56mw0000gn/T
[14:46:17]: Logged in as: Gio Lodi
[14:46:17]: Strings.xml file for Login Library downloaded to /var/folders/29/wn6913pn73gf2l7j5n4n56mw0000gn/T/strings.xml.
[14:46:17]: ------------------------------
[14:46:17]: --- Step: an_localize_libs ---
[14:46:17]: ------------------------------
[14:46:17]: Merging Login Library strings into WooCommerce/src/main/res/values/strings.xml
[14:46:17]: - Skipping enter_email_wordpress_com string
[14:46:17]: - Skipping enter_email_for_site string
[14:46:17]: - Skipping enter_site_address string
[14:46:17]: - Skipping default_web_client_id string
[14:46:17]: - Skipping login_notification_channel_id string
[14:46:17]: Done (0 added, 0 updated, 117 untouched, 5 skipped).
[14:46:17]: ----------------------------------------------
[14:46:17]: --- Step: android_download_file_by_version ---
[14:46:17]: ----------------------------------------------
[14:46:17]: Downloading library/src/main/res/values/strings.xml from Automattic/about-automattic-android at version 0.0.4 to /var/folders/29/wn6913pn73gf2l7j5n4n56mw0000gn/T
[14:46:18]: Strings.xml file for About Library downloaded to /var/folders/29/wn6913pn73gf2l7j5n4n56mw0000gn/T/strings.xml.
[14:46:18]: ------------------------------
[14:46:18]: --- Step: an_localize_libs ---
[14:46:18]: ------------------------------
[14:46:18]: Merging About Library strings into WooCommerce/src/main/res/values/strings.xml
[14:46:19]: Done (0 added, 0 updated, 27 untouched, 0 skipped).
[14:46:19]: ------------------------
[14:46:19]: --- Step: git_commit ---
[14:46:19]: ------------------------
[14:46:19]: $ git status WooCommerce/src/main/res/values/strings.xml --porcelain
[14:46:19]: Nothing to commit, working tree clean β
.
[14:46:19]: Cruising back to lane 'android complete_code_freeze' π
[14:46:19]: --------------------------------------------------------------------------
[14:46:19]: --- Step: Switch to android update_frozen_strings_for_translation lane ---
[14:46:19]: --------------------------------------------------------------------------
[14:46:19]: Cruising over to lane 'android update_frozen_strings_for_translation' π
[14:46:19]: ------------------------
[14:46:19]: --- Step: git_commit ---
[14:46:19]: ------------------------
[14:46:19]: $ git status ./fastlane/resources/values/strings.xml --porcelain
[14:46:19]: βΈ M fastlane/resources/values/strings.xml
[14:46:19]: $ git commit -m Freeze\ strings\ for\ translation ./fastlane/resources/values/strings.xml
[14:46:19]: βΈ [test/6742 b0a0fede9b] Freeze strings for translation
[14:46:19]: βΈ 1 file changed, 11 insertions(+), 1 deletion(-)
[14:46:19]: Successfully committed "["./fastlane/resources/values/strings.xml"]" πΎ.
[14:46:19]: Cruising back to lane 'android complete_code_freeze' π
[14:46:19]: -------------------------------------
[14:46:19]: --- Step: ensure_git_status_clean ---
[14:46:19]: -------------------------------------
[14:46:19]: $ git status --porcelain
[14:46:19]: Git status is clean, all good! πͺ
[14:46:19]: --------------------------------
[14:46:19]: --- Step: push_to_git_remote ---
[14:46:19]: --------------------------------
[14:46:19]: $ git push origin test/6742:test/6742 --tags
[14:46:28]: βΈ remote:
[14:46:28]: βΈ remote: Create a pull request for 'test/6742' on GitHub by visiting:
[14:46:28]: βΈ remote: https://github.com/woocommerce/woocommerce-android/pull/new/test/6742
[14:46:28]: βΈ remote:
[14:46:28]: βΈ remote: GitHub found 1 vulnerability on woocommerce/woocommerce-android's default branch (1 low). To find out more, visit:
[14:46:28]: βΈ remote: https://github.com/woocommerce/woocommerce-android/security/dependabot/8
[14:46:28]: βΈ remote:
[14:46:28]: βΈ To github.com:woocommerce/woocommerce-android
[14:46:28]: βΈ * [new branch] test/6742 -> test/6742
[14:46:28]: Successfully pushed to remote.
+------+----------------------------------------------------------+-------------+
| fastlane summary |
+------+----------------------------------------------------------+-------------+
| Step | Action | Time (in s) |
+------+----------------------------------------------------------+-------------+
| 1 | default_platform | 0 |
| 2 | is_ci | 0 |
| 3 | check_for_toolkit_updates | 2 |
| 4 | Switch to android localize_libraries lane | 0 |
| 5 | android_download_file_by_version | 0 |
| 6 | an_localize_libs | 0 |
| 7 | android_download_file_by_version | 1 |
| 8 | an_localize_libs | 0 |
| 9 | git_commit | 0 |
| 10 | Switch to android update_frozen_strings_for_translation | 0 |
| | lane | |
| 11 | git_commit | 0 |
| 12 | ensure_git_status_clean | 0 |
| 13 | push_to_git_remote | 9 |
+------+----------------------------------------------------------+-------------+
[14:46:28]: fastlane.tools finished successfully π
- [x] I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary. β N.A.
You can trigger optional UI/connected tests for these changes by visiting CircleCI here.
You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:
@AliSoftware
the diff of the strings.xml file didn't show any line gaining the a8c-src-lib XML attribute π€
Finally had a chance to look into this. I basically compared how the action behaves for WordPress Android vs here. I noticed two things:
- If I add
tools:ignore="UnusedResources"to a node that comes from a library, then the automation correctly adds thea8c-src-libattribute - If I remove this check (
return :found if lib_string_node =~ main_string_node) in the release toolkit helper, then all the strings from libraries get thetools:ignoreanda8c-src-libattributes
I'm not familiar with how localizing Android strings works so not sure where to go from here. The only thing I can think of is to edit WooCommerce/src/main/res/values/strings.xml adding tools:ignore="UnusedResources" where appropriate (which as far as I can see is for every string coming from a library?) However, the way I read the release toolkit logic below makes me think me that's something the automation should already be able to do, so maybe there's a refinement to do there? π€
if main_string_node.attr('tools:ignore').nil?
# No `tools:ignore` attribute; completely replace existing main string node with lib's one
add_xml_attributes!(lib_string_node, library)
Haven't had time to dig deeper in this yet, but given the expected behavior described in paaHJt-3fg-p2 I believe there might indeed be a bug in the toolkit. I'd expect the strings coming from the libs to be annotated with the a8c-src-lib attribute regardless if they have been overwritten by the app or have the tools:ignore attribute indeed.
But I'll need to look deeper into this to be sure which path of the code this goes thru and what's really happening. Planned to do that todayβ¦ but got a surprise hotfix to take care of so that shattered my plans for the day; hopefully I'll be able to check on my tomorrow π€
@mokagio any update on that?
I have to admit I'm a bit swamped with my projects right now so haven't had the occasion to circle back on this to understand if this lack of a8c-src-lib is a bug in the toolkit or in the config in this PR.
Do you plan to take a look at this to debug what might be happening? I just don't want this PR to go stale and be forgotten π
(and would love this L10n tooling work to be considered completed on WCA once this lands, I think?)
@AliSoftware lol, I was waiting/hoping you could read that code and instantly figure out where the issue was.
But rest assured I have not forgotten this. However, I cannot promise I'll get to it during this sprint.
π @mokagio @AliSoftware !
Pinging you on this as it was accidentally shown in a PR view of mine as I was searching for something, then I got curious. This is just a friendly reminder in case it was buried on top of other priorities. Are we suppose to follow-up on that at some point, wdyt? π€
We definitively should follow-up on that and finally finish up that workβI think that was the last piece of the puzzle before finally considering the whole Localization Tooling Project Thread to be done, but we never had time to go back to it.
@mokagio I think you said previously that you were planning to take point on finishing the investigation on this and get to the bottom of it to finally finish this PR? Personally my hands are pretty full with all the work needed in Tumblr anyway, so I'll let that to you if you don't mind π
Yeah nah. I have my hands full, too. I will get to this at some point, I suppose, but I'm not in a place to have the space to dig into the implementation details.
FWIW and future reference, I did try to look into it in the more quiet period in December, and couldn't wrap my head around why it works in WordPress but not here.
The only guess I have is that the inputs that WordPress was already formatted in such a way that met the expectation of the tool, while the one in Woo doesn't.
I had two things I would have liked to try:
- Write new tests using a meaningful subset of the inputs in this and WordPress repos, to see how they behave.
- Add new tests, duplicating existing ones, that replicate the XML attributes we see here, and see what they do.