CLDR-18451 Update TerritoryInfo references after v48 changes #5154
Conversation
conradarcturus
requested review from
AEApple,
btangmu,
macchiati,
pedberg-icu and
srl295
conradarcturus
temporarily deployed
to
cloudflare
GitHub Actions
Inactive
|
✨ This commit deployed to https://3069219d.cldr.pages.dev Changed Pages in the last commit: |
|
|
||
| - Number patterns that did not have a specific numberSystem (such as `latn` or `arab`) had been deprecated for many releases, and were finally removed. | ||
| - Additionally, language and territory data in `languageData` and `territoryInfo` data received significant updates to improve accuracy and maintainability [CLDR-18087] | ||
| - Additionally, language and territory data in `languageData` and `territoryInfo` data received significant updates to improve accuracy and maintainability [CLDR-18087] In particular, the `territories` attribute in `languageData` was deprecated and removed, as it was unclear and prone to misunderstanding. Implementations that used this data may need to adjust accordingly, using `territoryInfo`. |
There was a problem hiding this comment.
Need a period here before "In particular"?
| if (languageInCountryData.officialStatus.isMajor()) { | ||
| // Output will look like <language type="sw" territories="TZ"/> | ||
| // This is no longer saved to output | ||
| // It used to appear like: <language type="sw" territories="TZ"/> |
There was a problem hiding this comment.
This is mysterious. Does the code still serve a purpose? There can be a tendency to keep code around for historical reference and/or in case it might be useful again someday. However, that leads to crufty hard-to-maintain code. So I'd rather see the code removed if unnecessary, and otherwise a comment explaining why the code is here, rather than a comment saying what it used to do or doesn't do...
There was a problem hiding this comment.
I agree, it's not great to leave this code in. I started this refactor last year but I punted the more consequential work until I had a proper design to replace the old class completely with a new format.
Removing this code ended up affecting other classes that consume the data and ended up causing a big rewrite that I wasn't prepared. Would you like me to change the comment to instead say "this is not saved to the output but it is used in the BasicLanguageData class"?
There was a problem hiding this comment.
That sounds good. Also, maybe (optional!) there could be a ticket for the larger refactoring, and you could put in a TODO comment referencing that ticket. Unless that's just wishful thinking :-)
This is a followup from #5148 -- checking other references in code to languageData/language/territories and updating them. The biggest change is updating the release notes.
https://unicode-org.atlassian.net/browse/CLDR-18451
CLDR-18451
ALLOW_MANY_COMMITS=true