-
Notifications
You must be signed in to change notification settings - Fork 683
chore: remove Promise.allSettled
shim
#4003
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.
Looks good to me! I don't think you needed to push the shrinkwrap for this one since no dependencies changed. I don't think it will hurt anything, though.
src/packages/core/src/server.ts
Outdated
@@ -7,14 +7,6 @@ import { | |||
|
|||
import allSettled from "promise.allsettled"; |
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.
The promise.allsettled
package itself should be removed.
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.
The promise.allsettled
package itself should be removed.
Looks like it doesn't work in Node v12.0.0. Let's wait on this until after the-merge is in |
4516f15
to
e1b2930
Compare
@tenthirtyone let's merge this one after the merge is released (which should be later today). |
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 all looks good to me. If you merge develop
it'll kill off the node12 builds 🥳
@jeffsmale90 - should have been taken care of in 6268b36 |
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2022 Ganache Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
This was TODO'd to be removed if we bumped
typescript
to4.2.3+