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

Allow splitting/merging translations for Crowdin #4503

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

romanz
Copy link
Contributor

@romanz romanz commented Jan 15, 2025

This way, we can submit them separately to Crowdin, so that each layout could be translated independently.

Following #4489.

@romanz romanz requested a review from obrusvit January 15, 2025 10:08
@romanz romanz self-assigned this Jan 15, 2025
Copy link

github-actions bot commented Jan 15, 2025

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@romanz
Copy link
Contributor Author

romanz commented Jan 15, 2025

Depends on #4495, to be able to reuse translations.get_translation() function.

@romanz romanz marked this pull request as ready for review January 15, 2025 10:19
@romanz romanz added the translations Put this label on a PR to run tests in all languages label Jan 15, 2025
@romanz
Copy link
Contributor Author

romanz commented Jan 15, 2025

The CI failures above are due to 4148e87, which will be removed before merging #4495.

@romanz romanz requested a review from prusnak as a code owner January 15, 2025 19:45
@romanz
Copy link
Contributor Author

romanz commented Jan 16, 2025

3c44637 is added to resort the ??.json files according to key order, to avoid an unrelated diff when running python crowdin.py merge from 32c99f0 (since it also uses tdir.save_lang(lang, blob_json) to update the translations).

@romanz romanz force-pushed the romanz/translations branch from 27b8ec8 to 8cdeca7 Compare January 20, 2025 11:50
@romanz romanz requested a review from matejcik as a code owner January 20, 2025 11:50
@romanz romanz marked this pull request as draft January 20, 2025 11:52
@romanz romanz changed the base branch from romanz/translations to main January 20, 2025 15:08
@romanz
Copy link
Contributor Author

romanz commented Jan 21, 2025

Rebasing over #4516 and adding full split/merge functionality here.

@romanz romanz force-pushed the romanz/translations-split branch from 32c99f0 to 7a55e7c Compare January 21, 2025 06:42
@romanz romanz marked this pull request as ready for review January 21, 2025 06:59
@romanz romanz linked an issue Jan 21, 2025 that may be closed by this pull request
8 tasks
@romanz romanz changed the title feat(core): allow splitting translations into per-layout JSONs Allow splitting/merging translations for Crowdin Jan 21, 2025
@romanz
Copy link
Contributor Author

romanz commented Jan 22, 2025

Rebasing over latest main to resolve a conflict (after #4495 was merged):

$ git range-diff HEAD^.. origin/romanz/translations-split^..
-:  --------- > 1:  e9aca6861 docs: make sure changelog fragments end with a period
-:  --------- > 2:  7ee1259aa refactor(core): use layout name for translation font keys
-:  --------- > 3:  8bccc7fd0 chore(core): resort translation JSONs by keys
-:  --------- > 4:  1dae795a9 feat(core): support per-layout untranslated text strings
-:  --------- > 5:  206119953 feat(python): support per-layout translated text strings
1:  536fd6404 = 6:  536fd6404 feat(core): allow splitting/merging translations for Crowdin

This way, each layout could be translated independently.

[no changelog]
@romanz romanz force-pushed the romanz/translations-split branch from 7a55e7c to 536fd64 Compare January 22, 2025 08:55
@romanz
Copy link
Contributor Author

romanz commented Jan 22, 2025

Tested by manually modifying cs.json and making sure the changes propagate to and back from Crowdin files:

diff --git a/core/translations/cs.json b/core/translations/cs.json
index 970b02b17..44061b8e3 100644
--- a/core/translations/cs.json
+++ b/core/translations/cs.json
@@ -57,7 +57,12 @@
     "address__confirmed": "Adresa příjemce potvrzena",
     "address__public_key": "Veřejný klíč",
     "address__public_key_confirmed": "Veřejný klíč potvrzen",
-    "address__qr_code": "QR kód",
+    "address__qr_code": {
+      "Bolt": "QR kód B",
+      "Caesar": "QR kód C",
+      "Delizia": "QR kód D",
+      "Eckhart": "QR kód E"
+    },
     "address__title_cosigner": "Další podepisující",
     "address__title_receive_address": "Přijímací adresa",
     "address__title_yours": "Vaše",

image

Copy link
Contributor

@obrusvit obrusvit left a comment

Choose a reason for hiding this comment

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

Tested ACK

@romanz
Copy link
Contributor Author

romanz commented Jan 22, 2025

Thanks!

@romanz romanz merged commit b9c98db into main Jan 22, 2025
140 checks passed
@romanz romanz deleted the romanz/translations-split branch January 22, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translations Put this label on a PR to run tests in all languages
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

Enable Crowdin workflow for firmware translations
2 participants