-
Notifications
You must be signed in to change notification settings - Fork 384
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-17535 Update likely subtags data #3966
CLDR-17535 Update likely subtags data #3966
Conversation
Set to draft until I resolve the errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM so far.
The tests pass locally now, so crossing my fingers that they do on the server. |
Ping: would like to merge this this morning if possible. |
Updated the log of differences and silData (now using newest sil data) |
* Scripts that either have no known languages as yet (Cpmn) or are used for any language | ||
* (Brai). | ||
*/ | ||
public static final Set<String> SCRIPTS_WITH_NO_LANGUAGES = Set.of("Brai", "Cpmn"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"any language" would be good script metadata (and supplemental data). Zyyy, Zxxx in the same boat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, would be: Zyyy, Zxxx are disallowed in likely BTW. Otherwise ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good so far, a couplemore files to review
LstrType.region, | ||
new LocaleValidator.AllowedMatch("001|419"), | ||
LstrType.language, | ||
new LocaleValidator.AllowedMatch("und|in|iw|ji|jw|mo|tl")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this information that's already in supplementalMetadata? ex <languageAlias type="tl" replacement="fil" reason="legacy"/>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A superset is. We only want the common ones here.
new LocaleValidator.AllowedValid( | ||
null, | ||
LstrType.region, | ||
new LocaleValidator.AllowedMatch("001|419"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this needs to be here - we already have en-150 for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 150, but it not externally relevant (eg in likely)
FYI |
CLDR-17535
Update the likely subtags with the new tool.
Still to do (but probably after alpha)
The format has been cleaned up, and the diff will be hard to read, so look at the delta in #3958
ALLOW_MANY_COMMITS=true