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

refactor: switch to async iterators #88

Merged
merged 5 commits into from
Nov 14, 2019
Merged

refactor: switch to async iterators #88

merged 5 commits into from
Nov 14, 2019

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Sep 4, 2019

BREAKING CHANGE: Switch to using async/await and async iterators.

Needs

package.json Outdated Show resolved Hide resolved
BREAKING CHANGE: Switch to using async/await and async iterators.
@@ -1 +0,0 @@
'use strict'
Copy link
Member Author

Choose a reason for hiding this comment

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

As the example was not even complete and we already have a pubsub example on js-libp2p, I think that it is not worth to have another one to maintain here.

Ideally, we should look at https://github.com/libp2p/js-libp2p-examples as the way to go? Once I get to automation of the interop for js-libp2p, I will also look into adding CI jobs for examples and we should probably migrate all examples to there and like in the appropriate repos the examples

@vasco-santos vasco-santos force-pushed the refactor/async branch 3 times, most recently from e32f0e2 to 8b38471 Compare October 16, 2019 16:44
@vasco-santos vasco-santos marked this pull request as ready for review October 18, 2019 19:10
Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

Some minor feedback right now. The majority of feedback for the pubsub PRs is in the abstract module, libp2p/js-libp2p-pubsub#26. I also left comments that will affect this in the js-libp2p PR libp2p/js-libp2p#467

package.json Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@vasco-santos vasco-santos force-pushed the refactor/async branch 7 times, most recently from d7ae2ed to a1410d6 Compare November 1, 2019 10:22
@vasco-santos vasco-santos requested review from jacobheun and removed request for jacobheun November 14, 2019 13:55
Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

One minor thing but otherwise this looks good. Just a friendly reminder to release as a beta dist-tag when it's ready :)

README.md Outdated

```JavaScript
const FloodSub = require('libp2p-floodsub')

const fsub = new FloodSub(node)
const registrar = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just remove the registrar declaration here and make node that it is a libp2p Registrar? I know we don't have the docs flushed out for that yet, but we can point to that once it's done.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you are right! Added a comment and removed the declaration. Once we have the docs, I will add a pointer to them here

@vasco-santos vasco-santos merged commit 2c32d25 into master Nov 14, 2019
@vasco-santos vasco-santos deleted the refactor/async branch November 14, 2019 18:09
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.

3 participants