Skip to content

Conversation

@dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Feb 15, 2019

BREAKING CHANGE: This change replaces callbacks with async / await

Needs the following PRs:

@dirkmc dirkmc force-pushed the chore/callbacks-to-async branch 2 times, most recently from a3c7cfe to 3329f36 Compare February 15, 2019 18:29
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.

Thanks a lot for the PR @dirkmc !

Left a few comments in the review.

Besides that, can you rebase this PR, as we are not using Jenkins CI anymore and I just added the travis config? Also, add a BREAKING CHANGE message in the commit body please.

@dirkmc dirkmc force-pushed the chore/callbacks-to-async branch 2 times, most recently from c14c5d9 to 03ec3d3 Compare February 19, 2019 13:51
@dirkmc
Copy link
Contributor Author

dirkmc commented Feb 19, 2019

Thanks @vasco-santos, I followed your suggestions with one small exception that I replied to

@vasco-santos vasco-santos added the status/blocked Unable to be worked further until needs are met label Feb 19, 2019
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.

All in all, this looks good! Added 3 minor details / suggestions!

@dirkmc dirkmc force-pushed the chore/callbacks-to-async branch 2 times, most recently from ff5ed4e to c85a1b5 Compare February 19, 2019 19:19
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.

Looks good! Thanks @dirkmc

The CI is failing as a result of the peer-id being installed through branch uses promises. As a consequence, js-ipfs uses this version as well (but it needs to use the callback version for now). So, once peer-id is released with a breaking change and we install its version here, everything should be fine.

@vasco-santos
Copy link
Member

@dirkmc can you update this PR? I think all the dependencies are now ready

@dirkmc dirkmc force-pushed the chore/callbacks-to-async branch from 6e659b3 to 1cb0063 Compare July 18, 2019 15:32
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

@vasco-santos vasco-santos merged commit 89e9903 into ipfs:master Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/blocked Unable to be worked further until needs are met

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants