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

IterableDataProvider::supported_locales is a misnomer #4872

Open
sffc opened this issue May 6, 2024 · 1 comment
Open

IterableDataProvider::supported_locales is a misnomer #4872

sffc opened this issue May 6, 2024 · 1 comment
Labels
2.0-breaking Changes that are breaking API changes C-data-infra Component: provider, datagen, fallback, adapters
Milestone

Comments

@sffc
Copy link
Member

sffc commented May 6, 2024

Now that we have a more well-established notion of a "supported locale" (#58), we should rename this trait function.

I suggest: iter_locales, and maybe make it return an iterator (if not too hard).

@sffc sffc added C-data-infra Component: provider, datagen, fallback, adapters 2.0-breaking Changes that are breaking API changes labels May 6, 2024
@sffc sffc added this to the ICU4X 2.0 milestone May 6, 2024
@robertbastian
Copy link
Member

I believe this should be

fn supported_requests(&self) -> Result<HashSet<DataRequest>, DataError>

This is because after moving key attributes to the request, supported-locales is not enough to enumerate all supported requests. And if we're using the "requests" phrasing, this will not clash with the notion of supported locales. Also, for performance, I'd like this to return a HashSet instead of an impl Iterator, like we already do internally in datagen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0-breaking Changes that are breaking API changes C-data-infra Component: provider, datagen, fallback, adapters
Projects
Status: Small breakage (defer to end)
Development

No branches or pull requests

2 participants