Skip to content
This repository was archived by the owner on Feb 26, 2021. It is now read-only.

fix: ensure all addresses are multiaddr #16

Merged
merged 2 commits into from
Sep 7, 2016
Merged

fix: ensure all addresses are multiaddr #16

merged 2 commits into from
Sep 7, 2016

Conversation

dignifiedquire
Copy link
Member

No description provided.


exports = module.exports = Peer

function ensureMultiaddr (addr) {
if (addr instanceof multiaddr) {
Copy link
Member

Choose a reason for hiding this comment

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

This will fail in times there is a version difference. I know that ideally we have the latest version of multiaddr everywhere, but we can't be 100% sure of that. Would be better to do duck typing.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about we add Multiaddr.isMultiaddr to the multiaddr module?

Copy link
Member

Choose a reason for hiding this comment

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

I like that :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@daviddias
Copy link
Member

@dignifiedquire thank you :) Other than the instanceof, LGTM :)

@daviddias
Copy link
Member

@dignifiedquire could you rebase and finish up this PR? Thank you

@dignifiedquire
Copy link
Member Author

@diasdavid ready once the referenced PR is merged and released

@daviddias daviddias merged commit a3389f0 into master Sep 7, 2016
@daviddias daviddias deleted the fix/multiaddr branch September 7, 2016 23:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants