Repository navigation
Conversation
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: reviewed the full PR diff against its merge base at c521c68a, including resource lookup, formatting and remaining translation coverage. Feedback on this draft PR; this is a COMMENT review, not approval.
1 actionable finding — LOW: use Spanish wording for the hardware passphrase label/title.
When the app language is Spanish (es, es-rES, or b+es+419), the newly added hardware__passphrase_button and hardware__passphrase_title resolve to Portuguese “Frase de acesso”. These resources are read by HwPairedSheet.kt, HwPassphraseSheet.kt and HwPassphrasePromptSheet.kt, so the paired-device action and passphrase-entry screens would display Portuguese wording. Please replace both values in all three Spanish resources with reviewed Spanish copy that identifies the passphrase clearly. Example at the reviewed revision.
Evidence: source analysis; these keys were absent on the base revision. No device reproduction or key-handling defect is claimed.
Validation performed: all 676 additions preserve existing resource values; no added-key duplicate or placeholder/markup mismatch was detected. All 15 changed XML resources parse and compile with AAPT2 36.0.0.
Remaining AI-assisted follow-up
Against 1,598 English source keys, missing nonempty values per locale are:
| Locale | Missing |
|---|---|
| ar | 543 |
| es-419 | 512 |
| ca | 522 |
| cs | 515 |
| de | 521 |
| el | 536 |
| es | 527 |
| es-ES | 521 |
| fr | 522 |
| it | 517 |
| nl | 522 |
| pl | 517 |
| pt | 547 |
| pt-BR | 512 |
| ru | 510 |
Prioritize the supported Pubky/Paykit flows: all 9 Pubky-auth strings are missing in every locale, along with 72–74 of 77 subscription strings and 55–58 of 60 payment-request strings. Profile and contact namespaces still miss 75–84 of 90 and 39–45 of 48 respectively. These are full namespace counts, broader than the release-feature subset in the PR description.
The raw backlog includes potentially unused English keys; establish usage before bulk translation. Preserve existing values, validate placeholders/plurals/markup, and retain native-speaker review for payment, security and recovery wording. Hardware passphrase terminology also needs review across languages; matching English text in translation memory does not establish matching context.
The pre-existing whole-file overwrite risk in pull-translations.sh is unchanged here and should be addressed separately to protect repo-only translations. Follow-up: #1489. Companion: synonymdev/bitkit-ios#937.
Device/layout testing and native-speaker approval were not performed in this review. Build/E2E jobs are skipped on this draft; resource compilation is not a full app build.
@piotr-iohk Fixed in 8496e09 for all three Spanish resources. It was a spelling issue rather than a wording choice: "acesso" is Portuguese and Spanish is "acceso". The same spelling was already in The iOS companion has the same fix for |
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. |
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
[Low impact] Updates translated UI strings for version 2.6.0. The PR appears safe to merge, with non-blocking translation corrections recommended. Findings
SummaryAdds 676 missing translations across 15 locales, corrects Spanish passphrase spelling, and adds a changelog entry.
Reviews (1) · Last reviewed commit: "chore: correct spanish spelling of passp..." · Reviewed by Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8496e09a18
ℹ️ 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".
| <?xml version="1.0" encoding="utf-8"?> | ||
| <resources> | ||
| <string name="cards__backupSeedPhrase__title">Sichern</string> | ||
| <string name="btcpay__missing_mnemonic">Bitkit konnte deine Wiederherstellungsphrase nicht lesen.</string> |
There was a problem hiding this comment.
Sort newly added resources by name
Move this btcpay__... entry before the cards__... entries; the same ordering problem occurs in several other modified locale files, where newly added contacts__... blocks split existing common__... blocks. The repository explicitly requires new localizable resources to be added alphabetically, so all inserted keys should be repositioned accordingly.
AGENTS.md reference: AGENTS.md:L347-L348
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Left as is for now: this is the Transifex pull, which keeps its source ordering, and the ordering rule is a style matter for the maintainers rather than a rendering problem.
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Merges 676 Transifex translations into 15 Android locales; nothing existing is removed or reworded beyond the es / es-ES / es-419 refresh. Reviewed 8496e09, full tier. Reasoned from the diff and CI; the branch was not built here.
What I checked, and 2 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. All 15 locale files come out clean against values/strings.xml at the head.
CI: build, build-local, lint and detekt green; the e2e-tests-local shards were still running.
Ruled out
- Unescaped apostrophes breaking aapt: none in the added values, and the build job compiles the resources.
- The 7 removed es-419 / es-ES / es lines: they are replacements of the same keys, not deletions, so no locale loses a key.
Wording was not reviewed by a native speaker; the PR body already scopes that out.
Merge confidence: 4/5, every added string passes the placeholder and markup checks; the e2e shards had not finished.
|
Folded into #1492, which now targets master and carries this pull plus the AI-assisted translations and the review fixes, so 2.6.0 translations land in one PR per platform. @piotr-iohk your review findings here are fixed there (f28bb07, merged in fcf156f). |
Twin: synonymdev/bitkit-ios#937
Refs: #1489
Description
slashtags__*profile strings,other__coming_soon,profile__qr_scan_label).Out of Scope
hardware__passphrase_*reads "Passwort". Payment, security and recovery copy needs native review.Design
N/A — no UI changes.
Preview
N/A — translation-only change.
QA Notes
Journeys
N/A — no user-visible behaviour change.
Manual Tests
N/A
Automated Checks
strings.xml— files are well-formed