-
Notifications
You must be signed in to change notification settings - Fork 259
fix: improve error handling in post-install script #3385
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
PR author doesn't have a seat in Blar. You can change this in https://app.blar.io/settings |
🦜 Time to pond-er this PR! Diving in to see if the error handling swims smoothly. Starting the review now. |
Alright, this PR's got more fixes than a cat with a laser pointer! I'm diving in now—get ready for a review from your friendly neighborhood frog. |
This PR improves error handling in the post-install script by addressing multiple issues. It throws an error if MSVC is not found on Windows, defines the previously undefined 'version' variable, and ensures the process exits with code 1 when 'npm i' fails due to 'execa' errors, which can sometimes throw 'undefined' errors. These changes enhance the robustness and reliability of the installation process, fix an issue referenced as #3384, and ensure better error visibility and handling during build and setup procedures. Pull Request Impact: 0 🔄 File Changes Overview
📜 Blar InstructionsBlar Commands
Tags Explanation
|
✅ No debugger issues found 🐛 Review's done! 🚀 Check out the feedback and let me know if you need anything! – Blar |
@tusharmath any reason you feel this is incorrect?
|
Summary:
version
not defined in fileundefined
for errors: https://github.com/sindresorhus/execa/blob/main/docs/errors.md#exit-codeprocess.exit(1)
fornpm i
to fail.Issue Reference(s):
Fixes #3384
Build & Testing:
cargo test
successfully../lint.sh --mode=fix
to fix all linting issues raised by./lint.sh --mode=check
.Checklist:
<type>(<optional scope>): <title>