[PR #2579] [MERGED] resolver: drop comparison/ordering implementations for configuration types #3150

New issue

Closed

opened 2026-03-16 11:28:10 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:28:10 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/2579
Author: @djc
Created: 11/18/2024
Status: ✅ Merged
Merged: 12/10/2024
Merged by: @djc

Base: main ← Head: no-compare

📝 Commits (9)

f6d4969 resolver: remove PartialEq/Eq derives from ResolverConfig
26b5b2c resolver: remove obsolete total_cmp() impl
e629621 tests: require provided order where necessary
0f97b3f resolver: sort connections by query RTT directly
c90528d resolver: remove PartialOrd/Ord impls for NameServer
0b34d9f resolver: drop PartialOrd/Ord impls for NameServerStats
fcb070e resolver: drop PartialEq/Eq impls for NameServerStats
f75f2f3 resolver: drop PartialEq/Eq impls for name server config types
1c9b1f8 resolver: drop PartialEq/Eq impls for name server config types

📊 Changes

6 files changed (+41 additions, -106 deletions)

View changed files

📝 crates/resolver/src/config.rs (+1 -18)
📝 crates/resolver/src/name_server/name_server.rs (+1 -38)
📝 crates/resolver/src/name_server/name_server_pool.rs (+3 -1)
📝 crates/resolver/src/name_server/name_server_stats.rs (+14 -44)
📝 crates/resolver/src/system_conf/unix.rs (+13 -3)
📝 tests/integration-tests/tests/integration/name_server_pool_tests.rs (+9 -2)

📄 Description

I don't think there are obvious use cases for these implementations, and it would be nice to have the flexibility to introduce internal configuration/state that is not PartialEq/Eq/PartialOrd/Ord. I also don't think the implementations are obviously correct (even before #2569, which dropped the TlsClientConfig wrapper that papered over the inability to compare TLS configurations) or conceptually obvious.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/2579 **Author:** [@djc](https://github.com/djc) **Created:** 11/18/2024 **Status:** ✅ Merged **Merged:** 12/10/2024 **Merged by:** [@djc](https://github.com/djc) **Base:** `main` ← **Head:** `no-compare` --- ### 📝 Commits (9) - [`f6d4969`](https://github.com/hickory-dns/hickory-dns/commit/f6d496979481487973785edbd5455ab5229e8138) resolver: remove PartialEq/Eq derives from ResolverConfig - [`26b5b2c`](https://github.com/hickory-dns/hickory-dns/commit/26b5b2cfae27814a13328adff843136984aa2349) resolver: remove obsolete total_cmp() impl - [`e629621`](https://github.com/hickory-dns/hickory-dns/commit/e629621ca9c8cd733b51b47a79aad1b98de8e1cd) tests: require provided order where necessary - [`0f97b3f`](https://github.com/hickory-dns/hickory-dns/commit/0f97b3ff8c7e03fe960b720e599b9d6e6dc00fea) resolver: sort connections by query RTT directly - [`c90528d`](https://github.com/hickory-dns/hickory-dns/commit/c90528dcccb29d43de5a92ac999014ed57913ca2) resolver: remove PartialOrd/Ord impls for NameServer - [`0b34d9f`](https://github.com/hickory-dns/hickory-dns/commit/0b34d9fde0df3ca309b04c5235399946efd485b7) resolver: drop PartialOrd/Ord impls for NameServerStats - [`fcb070e`](https://github.com/hickory-dns/hickory-dns/commit/fcb070ee8d249f743116c445043b19a991821bba) resolver: drop PartialEq/Eq impls for NameServerStats - [`f75f2f3`](https://github.com/hickory-dns/hickory-dns/commit/f75f2f349acc669339df26968c09528a833b7d90) resolver: drop PartialEq/Eq impls for name server config types - [`1c9b1f8`](https://github.com/hickory-dns/hickory-dns/commit/1c9b1f8d089076aed32404b9637de17938ec4713) resolver: drop PartialEq/Eq impls for name server config types ### 📊 Changes **6 files changed** (+41 additions, -106 deletions) <details> <summary>View changed files</summary> 📝 `crates/resolver/src/config.rs` (+1 -18) 📝 `crates/resolver/src/name_server/name_server.rs` (+1 -38) 📝 `crates/resolver/src/name_server/name_server_pool.rs` (+3 -1) 📝 `crates/resolver/src/name_server/name_server_stats.rs` (+14 -44) 📝 `crates/resolver/src/system_conf/unix.rs` (+13 -3) 📝 `tests/integration-tests/tests/integration/name_server_pool_tests.rs` (+9 -2) </details> ### 📄 Description I don't think there are obvious use cases for these implementations, and it would be nice to have the flexibility to introduce internal configuration/state that is not `PartialEq`/`Eq`/`PartialOrd`/`Ord`. I also don't think the implementations are obviously correct (even before #2569, which dropped the `TlsClientConfig` wrapper that papered over the inability to compare TLS configurations) or conceptually obvious. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>