-
Notifications
You must be signed in to change notification settings - Fork 682
chore: remove Promise.allSettled shim
#4003
Conversation
MicaiahReid
left a comment
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
| serverOptionsConfig | ||
| } from "./options"; | ||
|
|
||
| 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.
davidmurdoch
left a comment
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). |
jeffsmale90
left a comment
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
typescriptto4.2.3+