unicode-org / icu4x

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

Discuss: Should LocalePreferences have public fields #5785

Open zbraniecki opened 2 weeks ago

zbraniecki commented 2 weeks ago

Issue: Not a fan of this having public fields. But that can be fixed after the PR lands

_Originally posted by @sffc in https://github.com/unicode-org/icu4x/pull/5729#discussion_r1831619407_

sffc commented 2 weeks ago

Duplicate of #5786

sffc commented 2 weeks ago

Actually let's keep this issue open so we have one discussion per thread.

sffc commented 2 weeks ago

So the main problem here is that DataLocale lives in icu_provider and it needs to be efficiently constructed from this type that lives in icu_locale_core.

Can we just move DataLocale into icu_locale_core?

My personal opinion is that I think it is silly and over-designed to try and keep the types exported from core crates as minimal as possible and put specialized types into other crates. I prefer "useful" core crates: ones that export types that its dependents need.

sffc commented 2 weeks ago

Conclusion:

LGTM: @Manishearth @robertbastian @sffc @zbraniecki