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-17868 v46 BRS: derived annotations, cldrmodify #3970

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Aug 19, 2024

CLDR-17868

I also fixed a tool issue that's in several of our tools: where missing empty parents aren't added. New class CLDRTreeWriter to support writing a tree full of data. It tracks additions and deletions and fills in missing items. Examples:

  1. if ca_ES_VALENCIA.xml is added, it will automatically write ca_ES.xml
  2. if you request to remove sr_Cyrl it wont' allow it since it has children, it will instead just write an empty file.

CLDRTreeWriter is AutoCloseable ;and must be closed after use.

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

macchiati
macchiati previously approved these changes Aug 19, 2024
Copy link
Member

@macchiati macchiati left a comment

Choose a reason for hiding this comment

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

Looks like the annotations/* ones were mostly because collation changed.

- new class CLDRTreeWriter - handle writing/deletion/removal properly
- updated ca_ES and sr_Cyrl
Copy link
Member

@macchiati macchiati left a comment

Choose a reason for hiding this comment

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

where missing empty parents aren't added

That is a good addition, for the main directory. Logically, all of the other directories are read in conjunction with main, so an empty file in main is sufficient.

So the tool (and files) needs to be changes to only create empty files, etc in the main directory.

@srl295
Copy link
Member Author

srl295 commented Aug 20, 2024

where missing empty parents aren't added

That is a good addition, for the main directory. Logically, all of the other directories are read in conjunction with main, so an empty file in main is sufficient.

So the tool (and files) needs to be changes to only create empty files, etc in the main directory.

That sounds reasonable, but I think other tooling and tests break with that assumption. Even this generator had a hard coded exception for sr_Cyrl, opting to keep it in place instead of removing it.

So TestCLDRFile.TestFileIds is wrong to require en_CA for en_CA_VALENCIA? I think we might need to note this to our users if we were to make a change in v46 and drop intermediate files.

Edit The spec doesn't actually mention 'main' being special this way. Could we merge this PR for now and have a follow up to correct the spec and the data?

@srl295 srl295 requested a review from macchiati August 20, 2024 19:19
@srl295
Copy link
Member Author

srl295 commented Aug 20, 2024

I'll file a ticket separately for the parent locale issue.

@srl295 srl295 merged commit 15c195f into unicode-org:main Aug 20, 2024
12 checks passed
@srl295 srl295 deleted the cldr-17868/derived-annotations branch August 20, 2024 20:02
@srl295
Copy link
Member Author

srl295 commented Aug 20, 2024

CLDR-17906 for further parent locale issues

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