-
Notifications
You must be signed in to change notification settings - Fork 20
Update ixdtf to icu4x main git branch #365
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
Conversation
@nekevss We would have to make a release before merging this. Otherwise, we may need to wait until a new version of ixdtf gets published. |
Yeah, it's been two weeks as is, so a release would probably be useful. So we should merge in what FFI we can, make a release, and then we can merge this. I don't expect the |
@robot-head There's a new release of |
Done! |
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 like this just needs a rebase / merge 😄 But looks good to me overall!
Just to note: this won't technically fix either of the issues attached, but it is a good baseline update overall.
To address, #226 the code on line 55 of parsers/timezone.rs
would need to be update to the IXDTF timezone parser. Primarily to use TimeZoneParser::parse_identifier
.
If you'd like to update on this PR, feel free. If not, that's fine too. This is still a good step forward!
I believe this is now done! |
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.
Awesome. Looks good to me!
Possibly fixes #230 and #226