-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
[Merged by Bors] - Upgraded to ICU 1.2 #2826
Conversation
Imported both deps directly. This is now just blocked on unicode-org/icu4x#3335 |
Just realized we can completely bypass the @sffc Can you either confirm or deny my assumptions? I can rollback to our old method if needed, just want to be sure since doing it this way simplifies our logic a lot. |
Test262 conformance changes
|
Codecov Report
@@ Coverage Diff @@
## main #2826 +/- ##
=======================================
Coverage 51.38% 51.38%
=======================================
Files 417 417
Lines 41330 41328 -2
=======================================
Hits 21238 21238
+ Misses 20092 20090 -2
... and 3 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Yes, you have a blanket impl that leverages as_deserializing and as_downcasting, so you can use unstable constructors. The only catch is that the unstable constructors can't read from deprecated data keys, which we did for the first time in this release when we split the LocaleExpander key in three. So unless you regenerate your data, LocaleExpander will fail to construct. Using the non-unstable constructors allows you to use the old data key (they will map it for you internally).
|
Oh, that's pretty important, since I'd like to give our users the option to upgrade their dataset when they want, instead of forcing them to regenerate when we bump This is still blocked on that PR then. |
I rebased this to the latest |
icu_locid_transform version 1.2.1 is released. This should unblock your upgrade |
Cargo.toml
Outdated
@@ -62,4 +62,4 @@ opt-level = 1 | |||
# Enables "fat" LTO, for faster benchmark builds | |||
lto = "fat" | |||
# Makes sure that all code is compiled together, for LTO | |||
codegen-units = 1 | |||
codegen-units = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
codegen-units = 1 | |
codegen-units = 1 | |
This looks good :) I would run a Thank you @sffc for your fast response! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran datagen but git didn't detect any diff. Everything good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! :)
bors r+ |
👎 Rejected by label |
bors r+ |
This PR upgrades ICU to 1.2. Unfortunately we still have some breaking changes, so this is being handled in unicode-org/icu4x#3332 Co-authored-by: jedel1043 <jedel0124@gmail.com>
Pull request successfully merged into main. Build succeeded: |
This PR upgrades ICU to 1.2.
Unfortunately we still have some breaking changes, so this is being handled in unicode-org/icu4x#3332