Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLDR-17483 add lld Ladin locale #3643

Merged
merged 5 commits into from
Apr 23, 2024
Merged

Conversation

DavidLRowe
Copy link
Contributor

CLDR-17483

  • This PR completes the ticket.

This PR adds lld base locale; ticket asks for additional variants to be added.

ALLOW_MANY_COMMITS=true

@DavidLRowe
Copy link
Contributor Author

@srl295 Can you tell me why this failed? In the log, I see:
Error: (TestCoverageLevel.java:1282) Error: : Language subtags that are in a CLDR locale's ID are in root (1): expected "", got "lld"

@srl295
Copy link
Member

srl295 commented Apr 19, 2024

@srl295 Can you tell me why this failed? In the log, I see:

Error: (TestCoverageLevel.java:1282) Error: : Language subtags that are in a CLDR locale's ID are in root (1): expected "", got "lld"

No idea. Files look good and we need a separate ticket to fix the error message, I can't make sense of it. @macchiati ?

@srl295
Copy link
Member

srl295 commented Apr 19, 2024

Update: it was just attributeValueValidity.xml - and an unclear message. Filed CLDR-16818 to fix message. and updated the xml here

srl295
srl295 previously approved these changes Apr 19, 2024
@DavidLRowe
Copy link
Contributor Author

build fails with:

Error:  (TestCoverageLevel.java:801)  Error: Comprehensive & no exception for path =>	//ldml/localeDisplayNames/languages/language[@type="lld"]

Any advice?

@srl295
Copy link
Member

srl295 commented Apr 19, 2024

build fails with:

Error:  (TestCoverageLevel.java:801)  Error: Comprehensive & no exception for path =>	//ldml/localeDisplayNames/languages/language[@type="lld"]

Any advice?

Looks like it needs to be added to https://github.com/unicode-org/cldr/blob/main/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestCoverageLevel.java#L527

I reviewed that line with "need to revisit next time" - can you collect a ticket with the issues found in adding a locale? it seems like we may have actually regressed.

- leftover questions from unicode-org#3230 CLDR-17042 remain
@srl295
Copy link
Member

srl295 commented Apr 20, 2024

@DavidLRowe see if that does it

@@ -524,7 +524,7 @@ public void TestCoverageCompleteness() {
+ "|gaa|gag|gan|gay|gba|gbz|gez|gil|glk|gmh|goh|gom|gon|gor|got|grb|grc|gsw|guc|gur|guz|gwi"
+ "|hai|hak|haw|hax|hdn|hif|hil|hit|hnj|hsn|hup|hur|iba|ilo|inh|izh|jam|jbo|jgo|jmc|jpr|jrb|jut"
+ "|kaa|kab|kac|kaj|kam|kaw|kbd|kbl|kcg|kde|ken|kfo|kgp|kha|kho|khq|khw|kiu|kln|kmb|koi|kos|kpe|krc|kri|krj|krl|kru|ksb|ksf|ksh|kum|kut|kwk|kxv"
+ "|lad|lag|lah|lam|lez|lfn|lij|lil|liv|lkt|lmo|lol|lou|loz|lrc|ltg|lua|lui|lun|luo|lus|luy|lzh|lzz"
+ "|lad|lag|lah|lam|lez|lfn|lij|lil|liv|lkt|lld|lmo|lol|lou|loz|lrc|ltg|lua|lui|lun|luo|lus|luy|lzh|lzz"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@macchiati what's the purpose of this list? (not clear from #3230 / CLDR-17042 )

@srl295 srl295 requested a review from macchiati April 20, 2024 01:09
@DavidLRowe
Copy link
Contributor Author

So additional changes were needed to
common/supplemental/attributeValueValidity.xml
and
tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestCoverageLevel.java
in order to pass the tests.

Should this information be added to the procedure document? Or are you saying that something else should change so that these additions are not needed?

@DavidLRowe DavidLRowe merged commit a6fbd0f into unicode-org:main Apr 23, 2024
10 checks passed
@srl295
Copy link
Member

srl295 commented Apr 23, 2024

So additional changes were needed to common/supplemental/attributeValueValidity.xml and tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestCoverageLevel.java in order to pass the tests.

Should this information be added to the procedure document? Or are you saying that something else should change so that these additions are not needed?

Add it to the doc for now, but I think it's a bug. I'd recommend opening an issue tracking these two changes so they can be fixed.

@DavidLRowe DavidLRowe deleted the CLDR-17483 branch April 26, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants