Skip to content

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

Closed
valenting wants to merge 0 commit intoservo:mainfrom
valenting:main
Closed

Revert "Reimplement idna on top of ICU4X (#923)"#942
valenting wants to merge 0 commit intoservo:mainfrom
valenting:main

Conversation

@valenting
Copy link
Copy Markdown
Collaborator

This reverts commit 3d6dbbb.

See #937 for reasons behind this backout.

@valenting valenting requested a review from Manishearth June 17, 2024 12:21
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 17, 2024

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 ⚠️
idna/src/punycode.rs 90.00% 3 Missing ⚠️
idna/tests/uts46.rs 86.36% 3 Missing ⚠️
idna/src/lib.rs 66.66% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #942   +/-   ##
=======================================
  Coverage        ?   81.76%           
=======================================
  Files           ?       21           
  Lines           ?     3549           
  Branches        ?        0           
=======================================
  Hits            ?     2902           
  Misses          ?      647           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@djc
Copy link
Copy Markdown
Contributor

djc commented Jun 17, 2024

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

@valenting
Copy link
Copy Markdown
Collaborator Author

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

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

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

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

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.

3 participants