Skip to content

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

Merged
merged 13 commits into from
Jul 4, 2025

Conversation

robot-head
Copy link
Contributor

@robot-head robot-head commented Jun 22, 2025

Possibly fixes #230 and #226

@jedel1043
Copy link
Member

@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.

@nekevss
Copy link
Member

nekevss commented Jun 22, 2025

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 ixdtf release to be too far behind. I'll see about putting up PRs for ixdtf and resb sometime this week.

@nekevss
Copy link
Member

nekevss commented Jul 3, 2025

@robot-head There's a new release of ixdtf. Would you be able to update this PR to the new version vs. using ICU4X's repository?

@robot-head
Copy link
Contributor Author

@robot-head There's a new release of ixdtf. Would you be able to update this PR to the new version vs. using ICU4X's repository?

Done!

Copy link
Member

@nekevss nekevss left a 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!

@nekevss nekevss mentioned this pull request Jul 4, 2025
@robot-head
Copy link
Contributor Author

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!

@robot-head robot-head requested a review from nekevss July 4, 2025 02:10
Copy link
Member

@nekevss nekevss left a 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!

@nekevss nekevss merged commit 5773df3 into boa-dev:main Jul 4, 2025
8 checks passed
@robot-head robot-head deleted the ms/update_ixdtf branch July 4, 2025 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UtcOffset should reject when the offset is subminute precision.
3 participants