search

servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.27k stars 317 forks source link

Revert "Reimplement idna on top of ICU4X (#923)" #942

Closed valenting closed 2 weeks ago

valenting commented 2 weeks ago

This reverts commit 3d6dbbb1dfc64c597745d5d6b97f2a8dd543c42b.

See #937 for reasons behind this backout.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 93.16940% with 25 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@dcfbed3). Learn more about missing BASE report.

Files Patch % Lines
idna/src/uts46.rs 94.31% 17 Missing :warning:
idna/src/punycode.rs 90.00% 3 Missing :warning:
idna/tests/uts46.rs 86.36% 3 Missing :warning:
idna/src/lib.rs 66.66% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #942 +/- ## ======================================= Coverage ? 81.76% ======================================= Files ? 21 Lines ? 3549 Branches ? 0 ======================================= Hits ? 2902 Misses ? 647 Partials ? 0 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

djc commented 2 weeks ago

Might be easier to only revert the url -> idna dependency to 0.5 for now?

valenting commented 2 weeks ago

Might be easier to only revert the url -> idna dependency to 0.5 for now?

https://github.com/servo/rust-url/blob/3d6dbbb1dfc64c597745d5d6b97f2a8dd543c42b/url/Cargo.toml#L29

idna = { version = "1.0.0", path = "../idna" }

That might be a better idea, but we'd have to stop referring dependencies by path. :thinking: