Skip to content

swapped dep to maintained package #896

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.

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

Merged
merged 7 commits into from
Mar 9, 2021

Conversation

TJKoury
Copy link
Contributor

@TJKoury TJKoury commented Mar 9, 2021

Solves this issue.

Went with a different library (es6-promisify) since the package maintainer's solution (use Node's native util.promisify) would require an additional shim for browser use.

@vasco-santos
Copy link
Member

Thanks for the PR @TJKoury
Can you update the expected bundle size https://github.com/libp2p/js-libp2p/blob/master/.aegir.js#L51 to 222, so that the CI works here?

@vasco-santos
Copy link
Member

@TJKoury it seems that a release of moving-average module broke the build. I will check it out to unblock this

@TJKoury
Copy link
Contributor Author

TJKoury commented Mar 9, 2021

@vasco-santos Is there a task for running these typescript checks locally? Also, what is aegir checking for the bundle size?

@vasco-santos
Copy link
Member

vasco-santos commented Mar 9, 2021

@vasco-santos Is there a task for running these typescript checks locally

npm run build

Also, what is aegir checking for the bundle size?

This github action to control no big increases in bundle size for the browser over time

@TJKoury
Copy link
Contributor Author

TJKoury commented Mar 9, 2021

That’s strange, I’ve run the build task a few times but no errors.

@vasco-santos
Copy link
Member

That’s strange, I’ve run the build task a few times but no errors.

I could replicate locally. Perhaps you need to remove package-lock and reinstall. Anyway, I already fixed that in #897 but I am seeing other new issues now

@TJKoury
Copy link
Contributor Author

TJKoury commented Mar 9, 2021 via email

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 f2f3619 into libp2p:master Mar 9, 2021
@TJKoury TJKoury deleted the promisify-es6 branch March 9, 2021 16:03
achingbrain added a commit that referenced this pull request Mar 26, 2021
Fixes a regression introduced by #896

`promisify-es6` was swapped for `es6-promisify` but the APIs of the
two modules are no the same so the NAT hole punching code broke.

Also changes port numbers to 0 in the tests as other processes can be
listening on those ports which causes spurious test failures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants