Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

fix: no more circular dependency, become a good block of libp2p #13

Merged
merged 5 commits into from
Jul 17, 2017

Conversation

daviddias
Copy link
Member

@daviddias daviddias commented Jul 16, 2017

Fix #9. Just one last thing, fix:

 1) KadDHT put - get:
   Uncaught TypeError: pi.id.toB58String is not a function
    at Swarm.dial (node_modules/libp2p-swarm/src/dial.js:22:25)
    at Network.sendRequest (src/network.js:143:20)
    at KadDHT._findPeerSingle (src/private.js:244:17)

by leveling up swarm.dial to also support PeerId instead of just PeerInfo.

@daviddias daviddias force-pushed the fix/circular-dep branch 2 times, most recently from 3c41724 to e795c8c Compare July 17, 2017 08:00
src/index.js Outdated

/**
* Number of closest peers to return on kBucket search
* Number of closest peers to return on kBucket search, default 20
Copy link
Member

Choose a reason for hiding this comment

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

this is not 20 in the code but 6

@@ -11,23 +11,26 @@ const timeout = require('async/timeout')
const retry = require('async/retry')
Copy link
Member

Choose a reason for hiding this comment

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

👎 for the rename

@daviddias daviddias requested a review from pgte July 17, 2017 09:26
Copy link
Contributor

@pgte pgte left a comment

Choose a reason for hiding this comment

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

Not too familiar with DHT internals, but apart from one comment, the javascript look solid :)

src/index.js Outdated
*/
constructor (libp2p, kBucketSize, datastore) {
constructor (swarm, options) {
assert(swarm, 'libp2p-kad-dht requires a instance of kad-dht')
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you mean "libp2p-kad-dht requires a instance of swarm"?

@daviddias
Copy link
Member Author

daviddias commented Jul 17, 2017

CI is failing due to previously reported bug: #8

Passes locally.

@daviddias daviddias merged commit 810be4d into master Jul 17, 2017
@daviddias daviddias deleted the fix/circular-dep branch July 17, 2017 14:33
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.

Circular dep with libp2p-ipfs-nodejs
3 participants