remove c++ based node internal dns module#6298
Conversation
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR removes the C++ DnsUtil class and its autogate guard, unconditionally using the Rust JSG-native DNS implementation.
Findings (ranked by severity):
-
[MEDIUM] Stale
//src/rust/dnsdependency innode-coreBUILD target —src/workerd/api/node/BUILD.bazel:78still has//src/rust/dnsinimplementation_deps, but the only consumer (dns.c++) was deleted. The crate itself (src/rust/dns/lib.rsline 1) says: "This file is a somewhat duplicate of src/rust/api/dns.rs, and will be removed once autogate is unnecessary." That time is now. Remove the dep line and consider deletingsrc/rust/dns/entirely. -
[LOW] Stale include in
node.h—src/workerd/api/node/node.h:21still includes<workerd/util/autogate.h>but no longer references any autogate types after this change. Can be removed. -
[LOW] Dead autogate entry —
RUST_BACKED_NODE_DNSinsrc/workerd/util/autogate.h:32andsrc/workerd/util/autogate.c++:28-29has no remaining callers. Consider removing it in this PR or a follow-up.
This review was generated by an AI assistant and may contain inaccuracies.
|
Review posted successfully on PR #6298. Here's a summary of the findings:
|
a88eaff to
c1c10ac
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6298 +/- ##
==========================================
- Coverage 70.64% 70.63% -0.02%
==========================================
Files 420 418 -2
Lines 111649 111564 -85
Branches 18368 18368
==========================================
- Hits 78880 78801 -79
+ Misses 21800 21794 -6
Partials 10969 10969 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
optional, non-blocking: I assume the autogate itself will be removed in a follow-up? |
Yes! |
rust-backed-dns autogate has been fully rolled out.