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.

Sign up for GitHub

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

Jump to bottom

Where should locale filtering take place in the data exporter? #3409

Closed
sffc opened this issue May 3, 2023 · 3 comments · Fixed by #3784
Closed

Where should locale filtering take place in the data exporter? #3409

sffc opened this issue May 3, 2023 · 3 comments · Fixed by #3784
Assignees
@sffc
Labels
C-data-infra Component: provider, datagen, fallback, adapters S-small Size: One afternoon (small bug fix or enhancement) T-techdebt Type: ICU4X code health and tech debt
Milestone
Priority Backlog ...

Comments

@sffc
Copy link
Member

sffc commented May 3, 2023

Datagen takes the --locales CLI or API argument. Each exporter implements IterableDataProvider based on all locales for which it has data (such as in CLDR-JSON). These sets of locales may be fully, partially, or non-overlapping. We should discuss the right place to perform this filtering.

See #3386 (comment)
@sffc sffc added C-data-infra Component: provider, datagen, fallback, adapters discuss Discuss at a future ICU4X-SC meeting labels May 3, 2023
@sffc sffc mentioned this issue May 3, 2023
Merged
@robertbastian
Copy link
Member

I think in the future this will become more complicated. When we introduce secondary DataLocale keys (as planned for currencies I think), more arguments than --locale might go into determining whether a DataLocale should be in the output. Given that such logic is key-specific, it should be in the IterableDataProvider implementations.

@sffc
Copy link
Member Author

sffc commented May 11, 2023

Discuss with:

Optional:

@sffc sffc added the discuss-triaged The stakeholders for this issue have been identified and it can be discussed out-of-band label May 25, 2023
@sffc
Copy link
Member Author

sffc commented Jul 5, 2023

Conclusion: someone such as @sffc can prototype the alternative (centralized locale filtering with per-key logic) and we decide in code review if it is better that way.

LGTM: @sffc @robertbastian

@sffc sffc added this to the Priority Backlog ⟨P3⟩ milestone Jul 5, 2023
@sffc sffc added T-techdebt Type: ICU4X code health and tech debt S-small Size: One afternoon (small bug fix or enhancement) and removed discuss Discuss at a future ICU4X-SC meeting discuss-triaged The stakeholders for this issue have been identified and it can be discussed out-of-band labels Jul 5, 2023
@sffc sffc self-assigned this Jul 5, 2023
@sffc sffc mentioned this issue Jul 31, 2023
Merged
@robertbastian robertbastian mentioned this issue Aug 4, 2023
Merged
@sffc sffc closed this as completed in #3784 Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sffc sffc

Labels
C-data-infra Component: provider, datagen, fallback, adapters S-small Size: One afternoon (small bug fix or enhancement) T-techdebt Type: ICU4X code health and tech debt
Projects
None yet
Milestone
Priority Backlog ⟨P3⟩
Development

Successfully merging a pull request may close this issue.

Moving datagen options to export robertbastian/icu4x
2 participants
@sffc @robertbastian