Emit a localized Android What's New title (#878) - #951
Merged
Conversation
appchangelog-gen wrote the Kotlin title as a raw literal, which the i18n gate rejects. Because that gate audits the whole tree, one generated line red-checked every open PR on code none of them touched. It happened on 9.2.0 and again on 9.2.1, and both times someone cleared it by hand afterwards. The generator now emits `title = uiString(R.string.<key>)` and writes the string itself, using the repo's existing scheme: the first six slugged words plus sha1(title)[:8]. Verified against the 9.2.1 key that is already shipping, and a test pins it there so a drift in either the hashing or the slugging fails rather than mints a key nothing translates. Translations come from an optional `whatsnew.title_locales` block in the release notes front-matter. A locale with no entry falls back to the English title and is named in a warning. That trade is deliberate: leaving the key out of a locale fails the same gate this exists to prevent, and an English title on a German card is visible and fixable where a red main after every release is neither. Swift is untouched — SwiftUI auto-extracts, so its literal title is correct. Items stay literals on both platforms; only the title ever failed. Reproduced the failure first (reverting 9.2.1's title to a literal gives "FAIL 1 NEW hardcoded literal at AppChangelog.kt:42") and confirmed generated output now passes the audit end to end. Also switched the Tools/ CI step from a named module to discover. It ran test_i18n_audit by name, so this PR's 14 new tests would have been added and never run — the exact hole #943 was meant to close.
CI caught this: the new tests import appchangelog-gen to exercise title_key and esc_xml, and the module did `import yaml` at load, so on a clean runner with no PyYAML the import failed and the suite errored. It passed locally only because PyYAML happens to be installed here. Moved the import into frontmatter(), the only function that needs it, with the same failure message. The pure helpers now import on any Python. Proven by running the suite with yaml blocked from sys.meta_path: 14 tests, 0 errors. The alternative was pip-installing PyYAML in the workflow, as fork-release.yml does. Not needing the dependency at all is better than installing it to test functions that never touch it. Note the count floor did its job here — 37 collected against a floor of 45 is what turned an import error into a red step rather than a quiet pass.
Found by doing the obvious thing: generate, edit the headline, re-run. apply() skipped an entry that already existed for that version, while write_title_strings ran unconditionally. So an edited headline minted a NEW key and wrote it into all six locale files, while the entry kept referencing the OLD one. The card showed the previous headline, the new key sat orphaned, and nothing failed — a silent wrong answer, which is worse than the red gate this PR is fixing. apply() now takes the platform's rendered title line and re-applies it to an existing entry, on both platforms so Kotlin and Swift cannot drift apart. A re-run with an unchanged headline is a no-op on the entry and still refreshes the constant, as before. Three regression tests. One of them was wrong on the first attempt — it asserted the whole file was byte-identical, when apply() legitimately rewrites the version constant every run. It now asserts the title line, which is the actual invariant.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #878, from @pipiche38's write-up — the diagnosis and the proposed key scheme are theirs.
The problem, reproduced before fixing
appchangelog-gen.pywrote the Kotlin title as a raw literal. Compose has no auto-extraction, so thei18n gate rejects it — and because that gate audits the whole tree, one generated line red-checks
every open PR on code none of them touched. It happened on 9.2.0 and again on 9.2.1, cleared by hand
both times.
Confirmed rather than assumed: reverting 9.2.1's title to a literal gives
Only the title ever failed. Items are long-form prose the gate doesn't require extracting, and Swift is
unaffected because SwiftUI auto-extracts — so both keep their literals.
The fix
The generator now emits
title = uiString(R.string.<key>)and writes the string itself, keyed by therepo's existing scheme (first six slugged words +
sha1(title)[:8]). Verified against the key alreadyshipping for 9.2.1, and a test pins it to that live artifact — so a drift in the hashing or the
slugging fails, rather than quietly minting a key nothing translates.
Translations come from an optional
whatsnew.title_localesblock in the release notes front-matter.The trade I want checked
A locale with no translation gets the English title, plus a named warning. The alternative — leaving
the key out of that locale — fails the very gate this exists to prevent. An English title on a German
card is visible and fixable; a red
mainafter every release, silently, because release pushes useGITHUB_TOKENand trigger no CI, is neither. Say if you'd rather it hard-fail instead; it is one line.Verified
Ran it end to end against a synthetic release file: correct key, correct
uiStringreference, stringswritten to all six locale files, apostrophes escaped for aapt2, warning naming the three locales I left
out — and
i18n_audit.py --cipasses on the result, which is the whole point. Reverted those artifacts;this PR touches only the tooling.
14 new tests, all passing.
One thing this PR also fixes about #943
That PR's
Tools/step rantest_i18n_auditby name, so these 14 tests would have been added andnever run — the exact hole #943 was meant to close. Switched to
discover, floor raised to 45.