Skip to content
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

fix!: Only resolve the first DNS-like component #61

Merged
merged 3 commits into from
Oct 1, 2024

Conversation

MarcoPolo
Copy link
Contributor

As far as I know, the ability to resolve multiple DNS addresses isn't used. This removes that ability for the simpler approach of just resolving the first DNS component in the multiaddr

BREAKING CHANGE: Previously Resolve would resolve all DNS components in a multiaddr. Now it will only resolve the first one. Users may iteratively call Resolve to get the old behavior. See the tests for an example.

resolve.go Show resolved Hide resolved
resolve_test.go Outdated Show resolved Hide resolved
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code panics for nil, returns an empty list on master. Should we keep the current behavior?

@MarcoPolo
Copy link
Contributor Author

Panics should be reserved for cases where you don't know what to do next and the best option is to stop the program. Returning an empty list for a nil address seems reasonable to me and seems better than stopping the program.

BREAKING CHANGE: Previously Resolve would resolve all DNS components in
a multiaddr. Now it will only resolve the first one. Users may
iteratively call Resolve to get the old behavior. See the tests for an
example.
@MarcoPolo MarcoPolo merged commit 25fce9e into master Oct 1, 2024
9 checks passed
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.

2 participants