chore(i18n): make config/locales.tsv the one language list (#262) - #335
Conversation
The language list lived in seven places — `ResourceParityTest` twice, `UntranslatedStringTest` twice, `noteLocales` in `app/build.gradle.kts`, `$StoreLocales` in two verify scripts, four lists in a third — plus counts in prose. A language missing from one of them was not a review problem: the surfaces it was missing from are exactly the surfaces nothing looks at, so it shipped untested and green. Chinese (#294) and Croatian (#329) both arrived that way, and both were caught by counting by hand. `config/locales.tsv` is now the list, and every consumer reads it: both resource tests through `LocaleManifest`, the store locales in the release export, `verify-release-notes.ps1`, `verify-release-export.ps1` and `verify-landing-versions.ps1` through `scripts/locales.ps1`, and the release-notes assertion in `.gitlab-ci.yml` through one `awk` line. The format is a four-column table for exactly that reason — Kotlin, Gradle, PowerShell and a shell one-liner all read it without a dependency. The list alone only closes half of it, so `scripts/verify-locales.ps1` compares the manifest against the files on disk in both directions — a declared language whose files are missing, and a file whose language is not declared — across app resources, starter notes, fastlane store directories, landing pages, privacy pages, READMEs and the demo clips. It runs in `build` on every push and pull request, and in GitLab's `verify_release_docs`. Three of the tracker's other items fall out of the same change: - **Nothing checked that a landing language has a privacy page.** Privacy pages carry no version string, so `verify-landing-versions.ps1` never walked them, and a language added without its translation gave two dead links per page. The new gate walks them with everything else; deleting `privacy.hr.html` fails it by name. - **`.gitlab-ci.yml` still grepped for six locale blocks.** It had been missing `zh-CN` since #294 and `hr-HR` since #329, unnoticed because the job is gated behind `GITLAB_RELEASE_FROM_CI` and nothing runs it — a gate that would pass the wrong thing the moment anyone turned it on. The path is kept (it is the documented fallback for GitHub being unavailable), so the list is derived rather than the job deleted. - **`verify-release-export.ps1` reported "여섯 로케일 블록" whatever the count was.** The assertion was right and the OK line said "six" through two additions. It now prints the count and the locales it actually checked. Prose counts are the one thing that cannot be derived, so the gate checks those too: `AGENTS.md`, `docs/RELEASE.md` and `docs/assets/README.md` are read for their language counts and compared with the manifest, and `AGENTS.md`'s `(en · ko · …)` list is compared as a set. The manifest is also declared as an input of the unit test tasks. Without that, editing it alone left `testDebugUnitTest` UP-TO-DATE — CI checks out clean and never saw it, but the person adding a language would have. Verified by breaking it in both directions: dropping the `hr` row fails 12 checks naming every `hr` file, and declaring a language with no files fails every surface it should. Both resource tests fail on the same drift once the manifest is an input. `./gradlew test` (all variants), the three verify scripts and `verify-release-export.ps1` against the real v2.33.0 artifacts in `D:\Build` are green, and the Gradle snippet was run against the manifest to confirm it yields the same eight store tags in the same order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![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: e800ab423e
ℹ️ 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".
| Write-Host "`n앱 리소스 (app/src/main/res)" | ||
| Test-Declared "strings.xml" @($locales | ForEach-Object { "app/src/main/res/$($_.ResDir)/strings.xml" }) | ||
| # values-night 처럼 언어가 아닌 한정자는 두 글자 코드 모양이 아니라 저절로 빠진다. | ||
| Test-Strays "values-*" "app/src/main/res" '^values-([a-z]{2})(?:-r[A-Z]{2})?$' $codes -Directory |
There was a problem hiding this comment.
When a values-en directory is accidentally added, this check accepts it because $codes includes the source locale en, even though the manifest contract maps the source language exclusively to values/. Android will prefer values-en on English devices, while ResourceParityTest continues reading only values/, allowing stale or incomplete source resources to bypass the new bidirectional gate. Compare suffixed directories against non-source locale codes instead; the analogous raw-* check has the same source-locale hole.
AGENTS.md reference: AGENTS.md:L266-L270
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 318f4f0 — correct on both counts, including the raw-* hole you pointed at. The suffixed surfaces (values-*, raw-*, index.<code>.html, privacy.<code>.html, README.<code>.md) now compare against the translated codes only, and a source-language file in one of those places gets its own message instead of the misleading "not in the manifest" one. Verified by adding values-en and index.en.html: both fail by name.
| $duplicates = @($locales | Group-Object Code | Where-Object { $_.Count -gt 1 }) | ||
| if ($duplicates.Count -gt 0) { | ||
| throw "$path 에 중복된 code 가 있습니다: $(($duplicates | ForEach-Object { $_.Name }) -join ', ')" | ||
| } |
There was a problem hiding this comment.
When a new manifest row accidentally reuses another row's store value, this parser accepts it because it checks only duplicate Code values. The locale verifier then satisfies both rows with the same existing fastlane directory, and the Gradle exporter writes the same <store> release-note block twice, so the new language can still ship without its own store metadata while every gate remains green. Validate Store uniqueness alongside Code uniqueness.
AGENTS.md reference: AGENTS.md:L266-L270
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 318f4f0 — the parser now checks Store uniqueness alongside Code. A row reusing ko-KR is refused before any surface is walked, so the shared-fastlane-directory pass you describe cannot happen. Verified by adding such a row.
Two holes in the new gate, both from the Codex review on #335. **values-en would have passed.** The stray check compared against every declared code, and the source language is declared, so a `values-en` directory read as a known language rather than as a file in the wrong place. Android prefers it on English devices while `ResourceParityTest` keeps reading `values/`, so stale English resources would render with no gate looking at them. The same hole was in `raw-*`, `index.<code>.html`, `privacy.<code>.html` and `README.<code>.md`: the suffixed surfaces now compare against the translated codes only, and a source-language file there gets its own message rather than the misleading "not in the manifest" one. **A repeated store tag would have passed too.** Only `Code` was checked for uniqueness, so two rows could share `ko-KR`: both would be satisfied by the one existing fastlane directory, and the release-notes TXT would carry the same block twice while the new language shipped with no store metadata of its own. Both verified by breaking them — `values-en` + `index.en.html` fail by name, a duplicate `store` value is refused by the parser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repository rule is that a work unit lands in HISTORY.md, and the v2.33.0 entry above still lists these five candidates as open — Codex caught the omission on #336. One entry covers the pass rather than one per PR, since #335 (the locale list and its gate) and #336 (the golden and the seam it needed) are the same piece of work against the same tracker section. No CHANGELOG entry: nothing in either PR is user-visible, and the 69 byte-identical goldens are the evidence for the one production change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Four items from the v2.33.0 section of the hardening tracker (#262), all one shape: the language list was copied into seven places, so a new language could ship with most of its surfaces ungated.
The list
config/locales.tsvis now the one list. Four whitespace-separated columns —code,store,source,starter— with the filename rules written in its header. It is a plain table rather than JSON because the consumers are Kotlin, Gradle, PowerShell and a shell one-liner, and none of them should need a parser dependency to read eight rows.Consumers updated to read it:
ResourceParityTest— two hardcoded listsLocaleManifest.translated/.hasStarterNotesUntranslatedStringTest—LOCALESand aLANGUAGE_COINCIDENCESrow per localeLocaleManifest.translatedCodes; an absent coincidence list now means no exemptionsnoteLocalesinapp/build.gradle.kts$StoreLocales×2, four lists inverify-landing-versions.ps1scripts/locales.ps1grep -q '^<xx-XX>$'lines in.gitlab-ci.ymlawkloop over the manifestThe gate
A list alone closes half of it — a declared language whose files are missing is still silent.
scripts/verify-locales.ps1compares the manifest against the files on disk in both directions, across app resources, starter notes, fastlane store directories, landing pages, privacy pages, READMEs and demo clips. It runs inbuildon every push and PR, and in GitLab'sverify_release_docs.It also checks the language counts written into
AGENTS.md,docs/RELEASE.mdanddocs/assets/README.md, and comparesAGENTS.md's(en · ko · …)list as a set. Prose counts are the one thing that cannot be derived from the list, which is why they had been stale through two additions.The three items that fall out of it
verify-landing-versions.ps1never walked them; a language added without its privacy translation gave two dead links per page. Removingdocs/privacy.hr.htmlnow fails the gate by name..gitlab-ci.ymlstill grepped for six locale blocks — missingzh-CNsince Create strings.xml for Chinese localization #294 andhr-HRsince Create Croatian (hr) translation files #329. The job is gated behindGITLAB_RELEASE_FROM_CIand nothing runs it, which is why the drift went unnoticed; it would have passed the wrong thing the moment anyone turned it on. The path is the documented fallback for GitHub being unavailable, so the list is derived rather than the job deleted.verify-release-export.ps1reported "여섯 로케일 블록" whatever the count was. The assertion iterated the real list and was correct; the OK line beneath it said "six" through two additions. It now prints the count and the locales it checked.One thing the change turned up
The manifest is declared as an input of the unit test tasks. Without it, editing
config/locales.tsvalone lefttestDebugUnitTestUP-TO-DATE, so the tests kept their previous verdict. CI checks out clean and would never have seen it; the person adding a language would have.Verification
hrrow → 12 failures naming everyhrfile across every surface, plus all four count checks.docs/privacy.hr.html→ the exact item-C case fails by name../gradlew test(all variants) green;verify-locales.ps1,verify-landing-versions.ps1,verify-release-notes.ps1green;verify-release-export.ps1green against the real v2.33.0 artifacts inD:\Build, now printing8개 로케일 블록이 모두 있습니다: ko-KR, en-US, ….Tracker: #262 (v2.33.0 section, four of five items).
🤖 Generated with Claude Code