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

Exported DNS format seems wonky #40

Closed
raulk opened this issue Feb 27, 2019 · 9 comments
Closed

Exported DNS format seems wonky #40

raulk opened this issue Feb 27, 2019 · 9 comments
Assignees
Labels

Comments

@raulk
Copy link
Member

raulk commented Feb 27, 2019

The exported DNS format allows for TCP encapsulation, which is weird: https://github.com/multiformats/js-mafmt/blob/master/src/index.js#L25

It was introduced in this PR: #14

@raulk
Copy link
Member Author

raulk commented Feb 27, 2019

IMO, addresses like /dns/google.com/tcp/80 belong in the Reliable format, not in DNS. CC @vasco-santos @jacobheun

@vasco-santos
Copy link
Member

I also agree that that address should not fall under the DNS format, but in Reliable...

@jacobheun
Copy link
Contributor

Yes, https://github.com/multiformats/js-mafmt/blob/v6.0.7/src/index.js#L25, should just go away. It's already covered in TCP which is already part or Reliable.

@vasco-santos
Copy link
Member

I will move that out then

@raulk
Copy link
Member Author

raulk commented Feb 27, 2019

Is there a reason DNS addresses are only supported on TCP? It seems they should be supported with other protocols, right?

@raulk
Copy link
Member Author

raulk commented Feb 27, 2019

This is what I ended up doing: multiformats/go-multiaddr-fmt@53297ec

@raulk
Copy link
Member Author

raulk commented Feb 27, 2019

@vasco-santos
Copy link
Member

This is what I ended up doing: multiformats/go-multiaddr-fmt@53297ec

I agree and will add it here!

@vasco-santos @jacobheun Another wonky entry: https://github.com/multiformats/js-mafmt/blob/master/src/index.js#L43

Thanks for the heads up, I will remove it!

@ghost ghost assigned vasco-santos Feb 28, 2019
@ghost ghost added the status/in-progress In progress label Feb 28, 2019
@jacobheun
Copy link
Contributor

Fixed in #42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants