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

Remove DynamicDataProvider and expand methods into ExportableProvider #4881

Closed
sffc opened this issue May 8, 2024 · 3 comments
Closed

Remove DynamicDataProvider and expand methods into ExportableProvider #4881

sffc opened this issue May 8, 2024 · 3 comments
Labels
2.0-breaking Changes that are breaking API changes C-data-infra Component: provider, datagen, fallback, adapters R-as-designed Resolution: By design principles, this won't be fixed S-small Size: One afternoon (small bug fix or enhancement)

Comments

@sffc
Copy link
Member

sffc commented May 8, 2024

I'm pretty sure that we're not using DynamicDataProvider anywhere except in the bounds of ExportableProvider.

We should merge DynamicDataProvider and IterableDynamicDataProvider into ExportableProvider in 2.0.

@sffc sffc added C-data-infra Component: provider, datagen, fallback, adapters S-small Size: One afternoon (small bug fix or enhancement) 2.0-breaking Changes that are breaking API changes labels May 8, 2024
@sffc sffc added this to the ICU4X 2.0 milestone May 8, 2024
@robertbastian
Copy link
Member

I see DynamicDataProvider as a core provider type, it's the equivalent to DataProvider where the key is only known at runtime. In fact, AnyProvider and BufferProvider should be instantiations of DynamicDataProvider for AnyMarker and BufferMarker.

@robertbastian robertbastian added the discuss Discuss at a future ICU4X-SC meeting label May 21, 2024
@sffc
Copy link
Member Author

sffc commented Jun 3, 2024

Also note the new trait BoundDataProvider which is similar to DynamicDataProvider but it is actually useful in components.

@robertbastian
Copy link
Member

DynamicDataProvider currently unifies code between BufferProvider aka DynamicDataProvider<BufferMarker>, and ExportableProvider, aka DynamicDataProvider<ExportMarker>. Ideally it would also do this for AnyMarker, but this doesn't currently work with yoke.

@sffc sffc closed this as completed Jun 27, 2024
@sffc sffc added R-as-designed Resolution: By design principles, this won't be fixed and removed discuss Discuss at a future ICU4X-SC meeting labels Jun 27, 2024
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 R-as-designed Resolution: By design principles, this won't be fixed S-small Size: One afternoon (small bug fix or enhancement)
Projects
Status: Done
Development

No branches or pull requests

2 participants