Repository navigation
chore: add 3.0.0 translations - #938
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a68e1a435
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
 [Low impact] Adds translated UI strings for new features. Update the fallback test before merging; the remaining translation fixes are small and non-blocking. Findings
SummaryAdds 8,706 translation entries across 14 locales, including a large expansion of Portuguese coverage.
Reviews (1) · Last reviewed commit: "chore: fill european portuguese translat..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
2 findings (2 P1). Adds AI-assisted and Android-reused translations so the 14 iOS locales cover every referenced 2.6.0 string, and fills European Portuguese from 52 to about 1,400 strings. Reviewed 0a68e1a against its base 98854b4 (#937). Reasoned from the diff and the Swift call sites; iOS does not build on this reviewer's Linux host.
What I checked, and 4 candidates I ruled out
Checked mechanically, every added or changed string: the file parses (Android: XML; iOS: every line is a "key" = "value"; entry), no duplicate keys, every key exists in English, and each value carries the same %1$s / {name} placeholders and the same markup tags as its English source. Only the two keys above differ from English.
Ran: node scripts/validate-translations.js at 0a68e1a: 0 errors. It does not compare placeholders, so it misses both findings.
CI: still running when reviewed.
Ruled out
settings__addr__spend_numberin es and pt dropping{fundsToSpend}or mixing English: no Swift code reads that key.- Arabic and pt
wallet__activity_input/_outputlosing{INPUT}/{OUTPUT}: in English those are plural branch text, not placeholders, and es onmasterhas the same shape. 50%in ptwallet__send_dialog2/_dialog3read as a format specifier:t()does noString(format:), so the%is printed as is.- Cold reader: both findings stood; it rated the copyright year P2, kept at P1 here because English renders the current year and these locales regress from it.
Wording was not reviewed by a native speaker; the PR body flags payment, security and recovery copy for that.
Merge confidence: 2/5, two P1s, each a one-line fix per locale.
ovitrif
changed the title
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against its merge base 68a1653, at 745ee30.
2 actionable findings — resolve or provide an evidence-backed rebuttal.
European Portuguese onboarding, receive, and passphrase-warning strings were copied from a different English source than the keys the screens load. The new Russian reset-graph confirmation contains a line break. Placeholder names, <accent> tags, and plural shape match English on the added and changed strings. Earlier {year} and <gray> comments are fixed on this revision.
Compared the affected strings with Android 74b17e7 (values-pt, values-ru). That was not a review of bitkit-android#1492. pt-BR in this PR matches the iOS English for the same keys; values-pt still has the stale sibling text.
Validation: node scripts/validate-translations.js at this commit reported 0 errors. It does not compare placeholders. Run Tests and validate passed on this commit. The Xcode suite was not rerun here.
Device testing: not performed in this review.
Suggested additional test cases
- iOS, language Português (Portugal): open onboarding. Slide 1 should mention profile, contacts, and borderless control; slide 2 should mention the Lightning Network; slide 3 should mention privacy and KYC.
- Same language: open the spending receive card. The body should be one sentence about instant, cheap bitcoin payments on Lightning, without mid-sentence line breaks.
- iOS, language Russian: open Recovery and the reset-graph alert. The destructive button should read “Да, Сбросить” on one line.
Findings
- [MEDIUM] Translate the European Portuguese strings the screens actually load — inline at
Bitkit/Resources/Localization/pt.lproj/Localizable.strings:416. - [LOW] Keep the Russian reset confirmation on one line — inline at
Bitkit/Resources/Localization/ru.lproj/Localizable.strings:677.
… strings and keep russian reset on one line
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up at bdec89fe, re-reviewing the delta since 745ee30 and the findings in baseline review 5531132914. Comparison base e40db96 and merge base 68a1653 are unchanged. This pass covered the six edited pt and ru strings, the screens that load them, and the Android sibling strings. Inherited baseline coverage is the other 12 locales, LocalizationPluralTests.swift, changelog.d/next/938.fixed.md, the unedited pt/ru strings, and the earlier placeholder, accent, plural, and newline comparison for every string this commit leaves as it was.
No new actionable code findings.
The European Portuguese onboarding alt slides, spending-receive sentence, and passphrase warning now follow the English those screens load. The Russian reset-graph confirmation is Да, Сбросить on one line. Placeholders, <accent> tags, and newline counts match English on these six keys. That matches the source for the Portuguese strings and the Russian button.
Android 74b17e7 values-pt still carries the older non-alt onboarding, receive, and passphrase copy, and values-ru security__reset_graph_confirm still contains a line break. That check is limited to those sibling strings and leaves bitkit-android#1492 for its own review.
Validation: node scripts/validate-translations.js at bdec89fe reported 0 errors. It does not compare placeholders; the six keys were compared separately. validate passed on this commit. The Xcode suite was not rerun here.
Device testing: not performed in this review.
Ready for device testing.
Suggested additional test cases
- iOS, language Português (Portugal): open onboarding. The first slide should mention profile, contacts, and borderless control; the second should mention the Lightning Network; the third should mention privacy and KYC.
- Same language: open the spending receive card. The body should be one sentence about instant, cheap bitcoin payments on Lightning, with no mid-sentence line breaks.
- Same language: open the backup passphrase warning. It should name the passphrase and include the keep-it-secret sentence.
- iOS, language Russian: open Recovery and the reset-graph alert. The destructive button should read “Да, Сбросить” on one line.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
One copy suggestion on the Portuguese widgets hint. The rest of the review follows with the device gate.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: device-QA follow-up, with the two heading concerns rechecked in source at 5db0233. Device observations below are from d12d5e2; this is not a fresh device pass or a full code reassessment of the master merge.
2 minor findings (P3): shorten the Polish subscriptions and German payment-history headings, or make long headings fit without splitting words. See inline comments.
Neither appears fixed by the head advance: both localized strings and empty-state layouts are unchanged. DisplayText gained optional spacing parameters, but these callers retain the same 44-point font, kerning and line-height defaults.
Device testing: iPhone 17 simulator, iOS 26, staging regtest, default text size. Checked subscriptions, payment history and named auth approval across 14 locales. Portuguese onboarding/receive/passphrase copy and widget hints, Russian reset confirmation, German QuickPay amount and French notification platform checks passed. Profile/Contacts coverage could not complete because the saved Pubky session failed to restore; the legacy auth sentence path was also unavailable. Native-speaker wording review remains separate.
Please recheck both headings at default size after the copy/layout fix. The current merged head has not had a fresh device run; this review is not a full device sign-off.
There was a problem hiding this comment.
Translation-length check on the keys this PR adds, run on iPhone 17 in el, es, ca, fr, ru, de, pt-PT, pt-BR, it, nl and en. Screens: Home, Settings General, Widgets, Hardware Wallets, Security, Data Backups. Inline: shorter translations for three keys that truncate or crowd the nav bar. All LOW; none hides an action, amount or fee.
The pt/pt-BR widgets hint from d12d5e2 fits.
Locale launch in ar, pt-PT and pl: no crash. Explorer plurals are correct (pl uses the "few" form for 2 outputs). Seen but not caused by this PR (ar.lproj already ships on master): ar amounts render digit groups in reverse order, chevrons and the back arrow do not mirror, and ActivityListFilter.swift:14 hard-codes "Search".
Device gate: bdec89f — iPhone 17 sim; locale launch ar/pt-PT/pl pass; Pubky auth sheet named variant pass (empty-clientID variant not reached); l10n: new keys on 6 screens checked in el/es/ca/fr/ru/de/pt-PT/pt-BR/it/nl, 3 keys overflow in 12 key/locale pairs (3 inferred from length, not captured). Later commits up to 97f20d4 change strings only on this branch, plus a master merge.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
Approved at e74f4a9 after the translation follow-up and current PR integration check. No new actionable code findings.
Reviewed the 19 copy changes since 97f20d4, the localized Activity search caller, English fallback plural test and the receive QR callback fix from #947. Current resource checks covered all 15 locale files: syntax passes, no duplicate or removed keys, and added/edited placeholder and markup contracts are preserved. Prior Portuguese, Russian and heading corrections remain present.
No additional device run was needed for this follow-up; this does not claim a fresh visual pass or native-speaker audit of every language. Build and validation passed when checked. Unit/integration/local E2E were still running at collection; approval does not claim those pending checks passed.
The merge-base changed after approval.
|
@piotr-iohk could you re-approve? GitHub dismissed your approval only because the merge base moved when #947 (the master build fix) merged; the head is still e74f4a9, the commit you approved, with no new changes. |
The merge-base changed after approval.
The merge-base changed after approval.
The merge-base changed after approval.
|
admin merged because something unclear kept removing the approval from @piotr-iohk . I will investigate later and find a resolution, likely caused by auto-merge ON + something in the ruleset |
4 participants
Closes #945
Closes #946
Closes #941
Twin: synonymdev/bitkit-android#1492
Refs: #936
Description
pull-translations.shlets the near-emptyarblocale overwritear.lproj(the script is unchanged here).{INPUT}/{OUTPUT}.acesso→accesoin the passphrase strings.pt.lproj), which had 52 strings: 799 come from the Androidvalues-pttranslations with identical English, and 575 are AI-translated in European Portuguese.Register and glossary per locale
Every locale reuses the terms its existing translations use most. Brand names stay in English: Bitkit, Pubky, Pubky Ring, Paykit, Lightning, Blocktank, USDT, BTCPay, Jade. The noun
pubkyalso stays in English.Out of Scope
wallet__payment_*(49)wallet__onchain_*(20)hardware__passphrase_*(11)wallet__send_*(7)profile__pay_*(4)hardware__jade_*(4)onboarding__restore_*(4)settings__quickpay__*(4)security__pin_*,security__mnemonic_*,settings__backup__*,onboarding__passphrase_*lightning__spending_confirm__*,lightning__savings_confirm__*,slashtags__error_*,settings__notifications__*subscriptions__payment_*,subscriptions__first_*,subscriptions__created_*,subscriptions__unsupported_*wallet__boost_*,wallet__lnurl_pay__*,wallet__toast_*,hardware__remove_*pubky_auth__description_*andpubky_auth__success_*: the sentence is built from prefix, service name and suffix, so most locales moved words into the prefix and left the suffix as ".". Each locale needs an on-screen check of the assembled sentence.hardware__passphrase_*reads "Passwort" while the rest of the app and the new strings use "Passphrase".slashtags__contact_pay_error; they were not reused.profile__your_namereads "Su Apellido" (surname); the new strings use "nombre".Design
N/A — no UI changes.
Preview
N/A — translation-only change.
QA Notes
Journeys
N/A — no user-visible behaviour change.
Manual Tests
Automated Checks
plutil -linton every changedLocalizable.stringsnode scripts/validate-translations.jsscripts/generate-plural-localizations.swiftover all locales<accent>tags and line breaks match the English