search

unicode-org / icu4x

Solving i18n for client-side and resource-constrained environments.
https://icu4x.unicode.org
Other
1.38k stars 178 forks source link

Tweaks on preference enum impls #5814

Closed sffc closed 1 week ago

sffc commented 1 week ago

Changes:

sffc commented 1 week ago

@zbraniecki I'm adding these impls because I'm trying to switch from icu::datetime::options::preferences::HourCycle to icu::locale::preferences::extensions::unicode::keywords::HourCycle

There's one other impl that is missing. HourCycle is a field of components::Bag which is used in fixture tests and therefore implements Serde. Should I:

  1. Implement Serde on all preferences enums (note: I was thinking of making the discriminant use the same string representation as BCP-47 via serde(rename), but this wasn't completely trivial because of Islamic, but I could probably make it work)
  2. Implement Serde on specifically the HourCycle enum
  3. Keep using a separate HourCycle enum in components::Bag
  4. Stop using components::Bag in Serde directly and instead use a fixture-specific wrapper (or remove it entirely: I don't think these fixtures are used any more)
sffc commented 1 week ago
  1. Stop using components::Bag in Serde directly and instead use a fixture-specific wrapper (or remove it entirely: I don't think these fixtures are used any more)

I'm doing this option

sffc commented 1 week ago

@zbraniecki I'm going to merge this because I don't think it's super controversial but if you have feedback we can do it in a follow-up