-
Notifications
You must be signed in to change notification settings - Fork 739
bump execa #1216
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
bump execa #1216
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1216 +/- ##
==========================================
- Coverage 97.47% 97.47% -0.01%
==========================================
Files 35 35
Lines 1468 1467 -1
==========================================
- Hits 1431 1430 -1
Misses 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Yeah, I'm aware of those. I'm working on updating ava which will fix everything I'm aware of. But let's leave that out of the scope of this PR. |
apply fixes on execa bump execa to v3 fix source and tests bump execa to v4 bump execa to v5 review comments Update src/cmd.js use standard posix 1 as code for default unknown error exit code fix after rebase
@nfischer all is fixed asrequested! |
I think you can fix the test failures with something like #1223. The critical part is inside |
Once you rebase, then I can retrigger CI to run |
i've fixed the new conflicts. |
Merged as e48e919 Thank you for the contribution! |
@nfischer thank you. i noticed the package.json version is not bumped nor a new version was published on the registry. is that a separate process? |
I push releases separately. I'm going to skim through some of the other GitHub issues and I expect I can release v0.10 shortly. |
Released in shelljs v0.10. Thanks for the contribution! |
following #1208
A few things to notice:
i've tried my best to bump the versions one by one major so that i could follow up and fix tests, propose a list of readable commits so that it's easier to review and follow. i wanted to go as far as i could into updating deps (more on that below), as usually keeping deps fresh is less prone to this kind of event (cve and such)
the fact that i had to provide a few fixes for execa@2 and @3 shows that an npm override wouldn't cut it and there's no further workaround than
patch-package
after this PR.i have hit a major barrier since execa moved to be a esm "only" module since v6. installing v6, running tests won't pass having issues such as:
that being said, execa@5 does not yield any entry in the audit report, so i guess it's good to leave it at v5.
After this point, I have tried to drop commits backwards and tested
npm audit --omit=dev
for every major and report says execa is free of vulnerabilities since its v2 (here) so if you prefer avoiding the fixes related to v3, i'm happy to drop the unnecessary commits and leave it at v2.I don't own a win32 machine to confirm that the change i made to the
commandNotFound
function is proper (v3 and above) so if possible to have a test made that'd be best.Not related to PR
when running
npm install
, stdout shows that a few dependencies are also marked as insecure bynpm audit
, mostly fromcross-spawn
widely used as well astrim-newlines
. they are devDependencies so it is less likely to be a problem (since production code should normallynpm i --prod
) so i think it's ok to leave them like that.