icu4x
icu4x copied to clipboard
Refactor: Move `MeasureUnit` to a Separate Crate
Task
Refactor the codebase by moving the MeasureUnit functionality into a dedicated crate.
Background
The MeasureUnit module is responsible for representing single or compound CLDR units (e.g., meter, square-meter, kilometer-per-second). This module is essential for various components, including unit formatting, unit conversion, and locale-aware unit representation.
To improve modularity and reusability across the project, it is necessary to isolate MeasureUnit into its own crate. This will facilitate better code organization and allow independent development and maintenance of unit-related functionalities.
Related issue: https://github.com/unicode-org/icu4x/issues/5336
- @younies - The crate represents units, including combined units. It can process "square-meter" and break it down into parts. It can also get the CLDR identifier for the unit. Currently I'm calling it "measure", but I'm not sure this is a good name.
- @sffc - It's similar to
icu_calendarandicu_timezoneandfixed_decimal. Not clear if it is a component or a utility. - @younies - It has its own data provider, the trie for parsing the unit string.
- @sffc - I recall that previously we discussed having
icu_unitsandicu_dimension. https://docs.google.com/document/d/1AftqfMIFjgQAKpIfO8pmYMZW-KIOBtYkVHZuwgYcfsw/edit - @sffc - We previously decided that the crate
icu_unitswould contain both the units conversion and the units structs. So we should first decide whether we want to split these crates.
A few ways we could split these crates:
icu_units(core data types),icu_units_conversion(conversion)icu_units(conversion and re-exports),icu_units_core(core data types). Could be extended to also split outicu_units_conversionicu_foo(core data types) andicu_bar(conversion)icu_units(re-exports),icu_units_parser(core data types),icu_units_conversion(conversion)
Discussion:
- @younies - We could have
icu_measure(core data types) andicu_units(conversion)... which is a little confusing - @sffc - That's basically my option 3.
- @sffc - Option 2 is consitent with what we did with
icu_locale. - @younies - A problem with option 2 is that most users will just want the data types. They don't need the conversion utilities.
- @echeran - There are users who don't need to optimize dependencies, etc. They want a top-level crate that has everything they need.
- @younies - The core data types do parsing, etc. It is its own use case.
- @sffc - The main downside of option 3 is that you need to invent new namespaces.
- @younies - Maybe option 4, since conversion is a narrow use case, so it should have a narrow crate.
- @sffc - Option 2 could be extended to have another child crate later. I prefer starting with fewer crates when possible.
- @younies - I already started the work to create the
icu_measurecrate. I'd like to finish doing that and fix it up to the agreement later. - @younies Does the data provider get split when we split out code from a crate into a smaller related crate?
- @sffc Yes.
icu_units_corevs.icu_measureis basically the same. - @younies In order to migrate
icu_units_core, do I also need to graduate the conversion? - @younies - Can we name it
icu_units_representationinstead oficu_units_core? - @sffc - We can add that to the bikeshed ballot. I think
icu_units_coreis best because it is consistent withicu_locale_corewhich the module structure is modeled after.
Proposal:
- Adopt option 2 above
- OK for Younies to make
icu_measureand then rename it toicu_units_bikeshedafter we decide the name. icu_units_bikeshedcan be graduated independently fromicu_units
LGTM: @younies @echeran @sffc
Based on the ballot, the winning crate name is: icu_units_core