-
Notifications
You must be signed in to change notification settings - Fork 298
Conversation
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.
Thanks @hugomrdias this LGTM 🚀, just one comment that needs to be addressed.
src/utils/load-commands.js
Outdated
@@ -68,7 +68,6 @@ function requireCommands () { | |||
cmds.util = (send, config) => { | |||
return { | |||
getEndpointConfig: require('../util/get-endpoint-config')(config), | |||
crypto: require('libp2p-crypto'), |
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.
README needs to be updated with this change and we need appropriate "BREAKING CHANGE" sections in the commits for this (as well as the switch to bignumber.js) so that they are reflected in the changelog when we release.
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.
@hugomrdias we need to do this please ^^
Note: #937 can be reverted after this is merged. |
38dc48a
to
766c5d4
Compare
This PR is ready to merge. We still have readable-stream duplication, but it will be fixed soon with deps updates like Tests are a bit flaky had to restart a few times to get everything green, i have some follow ups to improve this. |
Is this ready? There's still github deps in the |
i manage to improve some dependencies, bl and tar-stream got released today we still have concat-stream, ndjson and stream-http ndjson will probably be very difficult to get an upstream release |
fbe1f96
to
990e804
Compare
@alanshaw i think we only need the breaking change msg about crypto and bignumber |
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.
Crypto breaking change already happened in 30.0.0 😀 and bignumber.js in 29.0.0.
Before:
808 modules
size:
3 627 606 B
1 308 055 B minified
346.3 KB minified/gzip
bundle analyzer https://bundle-size.netlify.com/http-before.html
After:
493 modules
size:
1 919 990 B
779 949 B minified
218.06 KB minified/gzip
bundle analyzer https://bundle-size.netlify.com/http-after.html
-37.0314756% improvement in size
-38.985148515 % improvement in modules to load
resolves #828