-
Notifications
You must be signed in to change notification settings - Fork 1
fix: IPv6 detection #6
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR dynamically adjusts IPv6 detection and refactors code formatting, while also updating project packaging settings and CI workflows.
- Switch
IsIPv6Enabledto a getter that recomputes availability at runtime - Refactor
DnsClientBasemethod signatures and formatting for consistency - Enhance
.csprojwith symbols, source-link, and bump BouncyCastle; update GitHub Actions to pack and publish
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Dns/DnsClientBase.cs | Refactored method signatures, dynamic IPv6 check, formatting |
| ARSoft.Tools.Net.csproj | Added symbol support, updated package refs |
| ARSoft.Tools.Net.sln.DotSettings | Updated file header template for current year |
| ARSoft.Tools.Net.sln | Added ARM64/x64 configurations |
| .github/workflows/custom-build.yml | Changed build to pack and added publish step |
| List<DnsClientEndpointInfo> endpointInfos; | ||
| if (servers.Any(s => s.IsMulticast())) | ||
| { | ||
| var localIPs = NetworkInterface.GetAllNetworkInterfaces() |
Copilot
AI
Jun 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The localIPs variable is computed but never used. If the intent was to bind transports per local address, integrate localIPs into endpoint creation; otherwise, remove this unused variable.
| private static bool IsIPv6Enabled | ||
| { | ||
| get => IsAnyIPv6Configured(); | ||
| } | ||
|
|
||
| private static readonly IPAddress _ipvMappedNetworkAddress = IPAddress.Parse("0:0:0:0:0:FFFF::"); | ||
|
|
Copilot
AI
Jun 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each access to IsIPv6Enabled invokes IsAnyIPv6Configured(), which enumerates all network interfaces. Consider caching this result or limiting calls to avoid repeated expensive scans.
| private static bool IsIPv6Enabled | |
| { | |
| get => IsAnyIPv6Configured(); | |
| } | |
| private static readonly IPAddress _ipvMappedNetworkAddress = IPAddress.Parse("0:0:0:0:0:FFFF::"); | |
| private static bool IsIPv6Enabled => _isIPv6Enabled; | |
| private static readonly bool _isIPv6Enabled; | |
| private static readonly IPAddress _ipvMappedNetworkAddress = IPAddress.Parse("0:0:0:0:0:FFFF::"); | |
| static DnsClientBase() | |
| { | |
| _isIPv6Enabled = IsAnyIPv6Configured(); | |
| } |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
There is bug in ARSoft on when the IPv6 detection happens. This fixes that to allow us to switch between networks with different protocols