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

[WIP] Actually implement this #1

Merged
merged 1 commit into from
Apr 7, 2017
Merged

[WIP] Actually implement this #1

merged 1 commit into from
Apr 7, 2017

Conversation

@dignifiedquire
Copy link
Member

@dignifiedquire dignifiedquire commented Jan 2, 2017 •
edited by vasco-santos
Loading

The dragons still live here. So beware of them.

Things left to do

This was referenced Jan 17, 2017
Closed
Merged
@dignifiedquire dignifiedquire mentioned this pull request Feb 28, 2017
Closed
@flyingzumwalt flyingzumwalt mentioned this pull request Mar 29, 2017
Closed
10 tasks
This was referenced Apr 1, 2017
Merged
Merged
@daviddias daviddias mentioned this pull request Apr 2, 2017
Closed
daviddias
daviddias reviewed Apr 4, 2017
View reviewed changes
.travis.yml Outdated
@@ -0,0 +1,13 @@
language: node_js
node_js:
- 4
Copy link
Member

@daviddias daviddias Apr 4, 2017

Choose a reason for hiding this comment

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

6 and stable too

Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

update CI

daviddias
daviddias reviewed Apr 4, 2017
View reviewed changes
README.md Outdated
![](https://img.shields.io/badge/Node.js-%3E%3D4.0.0-orange.svg?style=flat-square)

> JavaScript implementation of the DHT for libp2p
> JavaScript implementation of the Kademlia DHT for libp2p, based on [go-kad-dht](https://github.com/libp2p/go-libp2p-kad-dht).
Copy link
Member

@daviddias daviddias Apr 4, 2017

Choose a reason for hiding this comment

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

s/go-kad-dht/go-libp2p-kad-dht

Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

update

@daviddias
Copy link
Member

daviddias commented Apr 6, 2017

@dignifiedquire seeing an error in occasionally:

  1) DHT find peer query:

      Uncaught AssertionError: expected 1 to deeply equal 0
      + expected - actual

      -1
      +0

      at utils.sortClosestPeers (test/index.spec.js:240:51)

daviddias
daviddias suggested changes Apr 6, 2017
View reviewed changes
src/index.js
* @param {number} [maxTimeout=10000] - how long to wait for the the bootstrap query to run, in milliseconds (10s)
* @returns {void}
*/
bootstrapStart (queries, period, maxTimeout) {
Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

This is, in fact, a discovery service that DHT provides and so as any discovery service it needs to be passed into the discovery mechanisms

Copy link
Member Author

@dignifiedquire dignifiedquire Apr 6, 2017

Choose a reason for hiding this comment

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

hmm

@daviddias
Copy link
Member

daviddias commented Apr 6, 2017 •
edited
Loading

Needs CR:

@dignifiedquire

@daviddias
Copy link
Member

daviddias commented Apr 6, 2017 •
edited
Loading

@dignifiedquire can you elaborate on the work behind (and importance of)

  • Fix all todos
  • ext_test.go
    • get failures
    • not found
    • less than K responses

@dignifiedquire
Copy link
Member Author

dignifiedquire commented Apr 6, 2017

@dignifiedquire can you elaborate on the work behind (and importance of)

These are nice to haves to check for specific failure behaviour but not crucial for shipping.

daviddias
daviddias suggested changes Apr 6, 2017
View reviewed changes
Copy link
Member

@daviddias daviddias left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • rm yarn.lock
  • create issues for the remaining items

.travis.yml Outdated
@@ -0,0 +1,13 @@
language: node_js
node_js:
- 4
Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

update CI

README.md Outdated
![](https://img.shields.io/badge/Node.js-%3E%3D4.0.0-orange.svg?style=flat-square)

> JavaScript implementation of the DHT for libp2p
> JavaScript implementation of the Kademlia DHT for libp2p, based on [go-kad-dht](https://github.com/libp2p/go-libp2p-kad-dht).
Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

update

README.md
### npm

```sh
> npm i libp2p-dht
Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

s/libp2p-dht/libp2p-kad-dht/g

README.md Outdated
### Use in Node.js

```js
const KadDHT = require('libp2p-dht')
Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

s/libp2p-dht/libp2p-kad-dht/g

README.md Outdated

## API

See https://libp2p.github.io/js-libp2p-dht
Copy link
Member

@daviddias daviddias Apr 6, 2017

Choose a reason for hiding this comment

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

s/libp2p-dht/libp2p-kad-dht/g

daviddias
daviddias suggested changes Apr 6, 2017
View reviewed changes
Copy link
Member

@daviddias daviddias left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • david finishes missing things

@daviddias daviddias mentioned this pull request Apr 6, 2017
Closed
@daviddias daviddias merged commit 0c036f8 into master Apr 7, 2017
@daviddias daviddias deleted the go-interop branch April 7, 2017 12:53
@daviddias daviddias mentioned this pull request May 19, 2017
Merged
35 tasks
@daviddias daviddias mentioned this pull request Aug 26, 2017
Open
@jacobheun jacobheun mentioned this pull request Apr 15, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@daviddias daviddias daviddias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dignifiedquire @daviddias