google / rust_icu

rust_icu: rust bindings for ICU (International Components for Unicode) library
Apache License 2.0
117 stars 28 forks source link

Set the resolver version to 2, clarify the error message when the ICU… #290

Closed clydegerber closed 1 year ago

clydegerber commented 1 year ago

… version isn't found and add implementations for ICU functions:

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

filmil commented 1 year ago

Your new tests are failing likely because the old ICU data does not have all the locale information present in more modern ICU versions, causing different results.

You may want to add a config flag on the value of icu_version_67_plus on the affected tests, causing them to be skipped when compiled with ICU v63.

https://github.com/google/rust_icu/blob/c6dac109bfd10645381e8ceeb23b2de86cc9e7e8/rust_icu_ecma402/src/listformat.rs#L30

(In general I recommend depending less on the exact text in the ICU data responses as ICU data tends to change frequently. But I admit this is something I did early on too.)

filmil commented 1 year ago

Merged. Thank you for your contribution!