-
Notifications
You must be signed in to change notification settings - Fork 14
Conversation
4485864
to
162c230
Compare
@jacobheun @vasco-santos any comments on this or can it be merged? |
I will have a look now @achingbrain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM!
Two observations:
@achingbrain can you add to the commit message the BREAKING CHANGE
, or @dignifiedquire please add it before squash and merge.
There are several errors coming from libp2p-crypto
, which should have a code. However, as this PR is for migrating to async await, we should do those changes in a next PR, but maybe we should create an issue for tracking this?
It's probably easy enough to just add a |
@achingbrain I agree. Just pointed out to not be forgotten 😄 |
@dignifiedquire any comments or can this be merged & released? |
looks good to me |
@dignifiedquire great! Could you merge & do a release please? I think only you and @daviddias have publish rights for this module on npm. |
This PR changes this module to remove callbacks and use async/await. The API is unchanged aside from the obvious removal of the `callback` parameter. depends on `multihashing-async` PR TODO refs ipfs/js-ipfs#1670 License: MIT Signed-off-by: Alan Shaw <[email protected]>
162c230
to
a334d77
Compare
0.4.0 is released with breaking change commit note. 🚢 |
This follows on from #9 (not sure why it's a PR from @alanshaw's personal account instead of a branch here 🤷♂) but fixes the merge conflicts and updates the deps.