-
Notifications
You must be signed in to change notification settings - Fork 409
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
Binary install update for axios security vulnerability. #1012
Conversation
Thanks for this! I will email @ashleygwilliams since hopefully she just has GitHub notifications disabled but hasn't fallen off the face of the earth. Edit: done, email sent. It probably won't hurt to tweet at her also, the Twitter handle is listed on her GitHub profile. I'll report back if I hear nothing in a few days. |
Do we also want to update the version number to
|
I'm pretty sure this is fixed in #983. |
I see, so we would need to merge that one before this one? |
There have been a couple of PRs to help clean up CI. I think fixing CI is outside the scope of this security fix. |
Ok, so that would mean we just merge even though CI is failing, correct? In other words, CI would fail if it were run on the current state of the |
It seems that way. The only PR since December to "pass" ci was #983 it seems. If whoever has merge and publish access to this repo wants CI to pass for this PR, I'm fine with fixing it but I'm not gonna look much more into it without someone showing up. |
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.
Thanks a lot!
Closes #958. Co-PR to #973.
Based on #973 (review), I've ran
node run.js
and verified that this works.If we'd like, I'm down to add a test subcrate that's runs
node run.js build
to actually run the full build. Thoughts?Make sure these boxes are checked! 📦✅
rustfmt
installedcargo fmt
on the code base before submitting✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨