-
Notifications
You must be signed in to change notification settings - Fork 9.6k
deps: remove unused c8 and update cross-spawn resolution #16845
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
base: main
Are you sure you want to change the base?
Conversation
|
I'd rather just update whatever dep is pulling this in. I think it's c8, which we don't really use anymore and should just remove. |
|
@connorjclark Agreed. If we're not using |
a80119f to
96348be
Compare
|
@connorjclark Done. I've stripped out |
KrrishSR4
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.
Reviewed the changes.
Removing the unused c8 dependency makes sense and effectively addresses the cross-spawn vulnerability while reducing technical debt. Keeping the resolution as a safety net also looks reasonable.
LGTM 👍
|
@KrrishSR4 Appreciate the feedback! |
I removed
c8and its related coverage configurations since we're not really using it anymore. This naturally resolves the security alerts caused by the oldercross-spawnversion thatc8was pulling in, while also cleaning up some technical debt.I've also kept the
cross-spawnresolution just to be safe in case any other transitive deps are still hitting it, but the primary fix was stripping out the unused dependency as suggested.