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

callbacks -> async / await #78

Merged
merged 7 commits into from
Sep 27, 2019
Merged

callbacks -> async / await #78

merged 7 commits into from
Sep 27, 2019

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Mar 15, 2019

BREAKING CHANGE: All places in the API that used callbacks are now replaced with async/await

@dirkmc dirkmc mentioned this pull request Mar 18, 2019
Closed
@jacobheun
Copy link
Contributor

There is an RFC for how peer discovery interfaces could be updated along with the change to async/await.

Ideally if we decided to go that route, getting a baseline of test suites at https://github.com/libp2p/interface-peer-discovery and including that here would be great.

daviddias
daviddias reviewed Sep 10, 2019
View reviewed changes
package.json Outdated Show resolved Hide resolved
@daviddias daviddias added status/blocked Unable to be worked further until needs are met and removed status/blocked Unable to be worked further until needs are met labels Sep 10, 2019
dirkmc and others added 2 commits September 27, 2019 10:43
@dirkmc @jacobheun
chore: callbacks -> async / await
abc3fab 
BREAKING CHANGE: All places in the API that used callbacks are now replaced with async/await

refactor(fix): clean up remaining async changes
@jacobheun
chore: fix lint
6713f08 
License: MIT
Signed-off-by: Jacob Heun <jacobheun@gmail.com>
@jacobheun
chore: update deps
59f9ff5 
License: MIT
Signed-off-by: Jacob Heun <jacobheun@gmail.com>
@jacobheun
chore: update CI file
26611e5 
License: MIT
Signed-off-by: Jacob Heun <jacobheun@gmail.com>
@jacobheun
Copy link
Contributor

I've rebased this off of master. I also removed libp2p-tcp as it was only being used for filtering. We can get the same effect by just using multiaddr.toOptions()

I also added node 12 to CI.

This should be good to go.

@jacobheun
test: add compliance tests
155602d 
License: MIT
Signed-off-by: Jacob Heun <jacobheun@gmail.com>
@jacobheun
Copy link
Contributor

@vasco-santos I added the new interface-peer-discovery tests, but there is an issue there libp2p/interface-peer-discovery#11 that this depends on.

@vasco-santos
Copy link
Member

I just released the fixed for interface-peer-discovery. Will review this now

@jacobheun
chore: update deps
72b1278 
License: MIT
Signed-off-by: Jacob Heun <jacobheun@gmail.com>
@jacobheun
Copy link
Contributor

I updated the interface version, should be all set pending the review

vasco-santos
vasco-santos suggested changes Sep 27, 2019
View reviewed changes
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

This looks good! Just pointed out 3 minor things

src/compat/querier.js Outdated Show resolved Hide resolved
src/query.js Outdated Show resolved Hide resolved
src/query.js Outdated Show resolved Hide resolved
@jacobheun
chore: resolve feedback
6da1a9e 
License: MIT
Signed-off-by: Jacob Heun <jacobheun@gmail.com>
vasco-santos
vasco-santos approved these changes Sep 27, 2019
View reviewed changes
Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM

@jacobheun jacobheun merged commit 46d78eb into libp2p:master Sep 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@daviddias daviddias daviddias left review comments

@vasco-santos vasco-santos vasco-santos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dirkmc @jacobheun @vasco-santos @daviddias