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

Not requiring --use-separate-crates for compiled data #3847

Merged
merged 6 commits into from
Aug 14, 2023

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented Aug 11, 2023

Replacing compiled data should not require extra icu4x-datagen flags. This also aligns the config.json more with the CLI arguments (it previously inverted the flag as use_meta_crate to get a true default).

#2945
#3564

@Manishearth
Copy link
Member

I'm not opposed to this change but I think I'm missing something about the motivation: "Replacing compiled data should not require extra icu4x-datagen flags". Can you share more details?

We probably should document how to use this; personally I find the non metacrate solution to be more intuitive and less likely to make people try and create cyclic deps.

@robertbastian
Copy link
Member Author

robertbastian commented Aug 14, 2023

The default datagen mode is using meta crate names, and I want compiled data to work with that. I think that's a reasonable default, because we tell our clients in all our tutorials to use the meta crate.

This change makes individual crates like icu_list usable with compiled data generated using meta crate names. What won't work is using baked output to define a custom DataProvider, this will require using either the meta crate or the --use-separate-crates flag. Using databake output like this is a very niche use case now that we have compiled data.

@robertbastian robertbastian changed the title Not requiring --use-separate crates for compiled data Not requiring --use-separate-crates for compiled data Aug 14, 2023
@Manishearth
Copy link
Member

The default datagen mode is using meta crate names

ah, right, forgot about that

+1

@robertbastian robertbastian merged commit 3dee486 into unicode-org:main Aug 14, 2023
24 checks passed
@robertbastian robertbastian deleted the meta branch August 14, 2023 18:30
@sffc sffc removed their request for review August 14, 2023 19:28
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