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

Move the registry into its own crate #5010

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented Jun 5, 2024

@sffc
Copy link
Member

sffc commented Jun 5, 2024

I thought the registry was going to the icu crate?

@robertbastian
Copy link
Member Author

It leads to a circular dependency for the broken-out datagen provider, and it's also pretty inefficient to pull in all of ICU4X just for some tokens.

@sffc
Copy link
Member

sffc commented Jun 5, 2024

It leads to a circular dependency for the broken-out datagen provider,

How? icu shouldn't depend on anything in datagen.

and it's also pretty inefficient to pull in all of ICU4X just for some tokens.

The tokens are symbols of the icu crate and can't really be used without including the icu crate.

@robertbastian
Copy link
Member Author

The tokens are symbols of the icu crate and can't really be used without including the icu crate.

It's entirely feasible to write a datagen script that uses only one subcomponent. It shouldn't pull in all of ICU4X.

Please let me do the refactorings, if we can identify a crate to fold it into after everything is done, Im happy to do that. It might be icu_provider.

@robertbastian
Copy link
Member Author

How? icu shouldn't depend on anything in datagen.

icu > icu_list > icu_provider_baked > icu. The last link is with the export feature only, but that doesn't matter for circularity.

@sffc
Copy link
Member

sffc commented Jun 5, 2024

How? icu shouldn't depend on anything in datagen.

icu > icu_list > icu_provider_baked > icu. The last link is with the export feature only, but that doesn't matter for circularity.

What is icu_provider_baked?

@sffc
Copy link
Member

sffc commented Jun 5, 2024

The tokens are symbols of the icu crate and can't really be used without including the icu crate.

It's entirely feasible to write a datagen script that uses only one subcomponent. It shouldn't pull in all of ICU4X.

You don't need the registry; it's mainly useful when you need to branch over all keys markers. But if you need to write a custom datagen script for just a few keys markers, you can just hardcode them.

Please let me do the refactorings, if we can identify a crate to fold it into after everything is done, Im happy to do that. It might be icu_provider.

ok

@robertbastian
Copy link
Member Author

baked_exporter needs the registry

@robertbastian robertbastian force-pushed the registry branch 2 times, most recently from 3346421 to 18714a6 Compare June 6, 2024 12:33
@robertbastian robertbastian marked this pull request as ready for review June 6, 2024 15:52
@robertbastian robertbastian requested review from a team, sffc and Manishearth as code owners June 6, 2024 15:52
@robertbastian robertbastian merged commit a3c242b into unicode-org:main Jun 7, 2024
28 checks passed
@robertbastian robertbastian deleted the registry branch June 7, 2024 08:32
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.

None yet

2 participants