-
Notifications
You must be signed in to change notification settings - Fork 446
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
feat: coalescing dial support #518
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.
I think this is in a good direction. Just found something that I did not understand
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.
LGTM! 🎉
Only, left a small suggestion for code readability
src/peer-store/index.js
Outdated
* @return {PeerInfo} | ||
*/ | ||
put (peerInfo) { | ||
put (peerInfo, silent = false) { |
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.
what do you think on transforming silent in an object? I think it helps to understand in the peerStore.put(peerInfo, true)
what the true
is
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.
Sounds good, I refactored and pushed a change to make it take an options object.
* docs: fix spelling in api * fix: dont create peerstore twice * feat: add support for dial coalescing * doc(fix): add setPeerValue to API TOC * docs: add more jsdocs to dialer * chore: remove old comment * fix: ensure connections are closed * fix: registrar.getConnections returns first open conn * fix: directly set the closed status * chore: remove unneeded log * refactor: peerStore.put takes an options object
connectToMultiaddr
, all dials now go directly throughconnectToPeer
PeerStore.put
now allows you to update a peer without firing thepeer
event. This helps prevent duplicate dialing when dialing a new multiaddr directly. (For example:ipfs swarm connect [addr]
)