Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: async await #87

Merged
merged 1 commit into from
Jul 11, 2019
Merged

feat: async await #87

merged 1 commit into from
Jul 11, 2019

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Oct 30, 2018 •
edited by alanshaw
Loading

More background about this effort: ipfs/js-ipfs#1670.

Despite some createX function not needing async, I added it to all of them to make them consistent. Otherwise we would have half returning a Promise and another half returning a PeerId

@ghost ghost assigned hacdias Oct 30, 2018
@ghost ghost added the status/in-progress In progress label Oct 30, 2018
@hacdias hacdias mentioned this pull request Oct 30, 2018
Closed
alanshaw
alanshaw suggested changes Oct 30, 2018
View reviewed changes
Copy link
Member

@alanshaw alanshaw 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.

I think its ok for the synchronous methods to be synchronous. Making them async will slow them down unnecessarily as it'll force the resolved value onto a future tick.

I understand the desire for them to all be consistent but a) right now they are not, b) it'll slow things down and c) it'll be an even bigger breaking change (and we've already got a LOT of work to do)

P.S. this is awesome - thank you ❤️

src/bin.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@hacdias
Copy link
Member Author

hacdias commented Oct 30, 2018

@alanshaw just made the updates 😄 I'll also take a look at peer-info later today!

@hacdias hacdias mentioned this pull request Oct 30, 2018
Merged
@hacdias
Copy link
Member Author

hacdias commented Oct 31, 2018

Done @alanshaw!

This was referenced Feb 18, 2019
Merged
Merged
@hacdias
Copy link
Member Author

hacdias commented Feb 25, 2019

@alanshaw should we re-do this or close?

@dirkmc dirkmc mentioned this pull request Mar 15, 2019
Merged
2 tasks
@kumavis kumavis mentioned this pull request Apr 22, 2019
Closed
@achingbrain
Copy link
Member

Rebased & merge conflicts resolved. Just need a release of libp2p/js-libp2p-crypto with libp2p/js-libp2p-crypto#131 in it and this should be good to go.

@alanshaw alanshaw changed the title [WIP] feat: async await feat: async await Jul 11, 2019
@alanshaw
Copy link
Member

Ready to merge \o/

@alanshaw
Copy link
Member

Needs BREAKING CHANGE message in squash commit.

@hacdias @achingbrain
feat: async await
24923c9 
BREAKING CHANGE: API refactored to use async/await

fix: remove async from non-async methods
fix: lint errors, use Boolean instead of !!
feat: re-add some timeouts to the tests
chore: rebase branch
chore: update libp2p-crypto dep

License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@achingbrain
Copy link
Member

Commit message updated to have BREAKING CHANGE warning.

jacobheun
jacobheun approved these changes Jul 11, 2019
View reviewed changes
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.

Changes look good

@jacobheun
Copy link
Contributor

@pgte @daviddias can we get a minor release of this? 🙏

Perhaps it makes sense for @vasco-santos or I to take over lead maintainer on this, peer-info and peer-book?

@vasco-santos vasco-santos mentioned this pull request Jul 11, 2019
Merged
@daviddias
Copy link
Member

daviddias commented Jul 11, 2019 •
edited
Loading

@jacobheun done :)

Perhaps it makes sense for @vasco-santos or I to take over lead maintainer on this, peer-info and peer-book?

It does! Thank you 👍🏽

@jacobheun jacobheun merged commit c3463c7 into master Jul 11, 2019
@jacobheun jacobheun deleted the feat/async-await branch July 11, 2019 17:09
@jacobheun
Copy link
Contributor

0.13.0 is out and awaits you 😉

@daviddias daviddias mentioned this pull request Sep 10, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobheun jacobheun jacobheun approved these changes

@alanshaw alanshaw alanshaw approved these changes

Assignees

@hacdias hacdias

Labels
status/in-progress In progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hacdias @achingbrain @alanshaw @jacobheun @daviddias