-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix: ignore node_gyp_bins if it exists #1391
Conversation
Thanks for opening a pull request! Here are some highlighted action items that will help get it across the finish line, from the
Development and triage is community-driven, so please be patient and we will get back to you as soon as we can. |
hash: '27f2dba4273f6c119000ec7059c27d86e27306d5dbbb83cfdfb862d92c679574' | ||
hash: '2ef4c03f6fc8adf03f782b40fddcfcdd26d1f3f3669ecedffeea5de5d28897c3' |
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.
I'm oretty sure that this hash changed because I updated the basic
app that baseOpts
is passing into the test. Tested this locally on an x64 machine, and the hash is consistently returning this value both locally and in CI. However, if this doesn't seem to pass the sniff test, let me know, happy to take another look.
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.
I'm oretty sure that this hash changed because I updated the basic app that baseOpts is passing into the test.
Might want to add a comment to this effect for future folks (and also how to find out the new value).
Hmmm. Separate PR to fix the xcode version in use, so the tests pass? |
@malept Yeah, hilariously that image failure is new as of this afternoon's rerun 😄 I'll fix that in another PR and then rebase |
83a84f6
to
47f5b4a
Compare
47f5b4a
to
8da6667
Compare
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.
Aside from a maintenance comment, this seems fine.
hash: '27f2dba4273f6c119000ec7059c27d86e27306d5dbbb83cfdfb862d92c679574' | ||
hash: '2ef4c03f6fc8adf03f782b40fddcfcdd26d1f3f3669ecedffeea5de5d28897c3' |
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.
I'm oretty sure that this hash changed because I updated the basic app that baseOpts is passing into the test.
Might want to add a comment to this effect for future folks (and also how to find out the new value).
Codecov Report
@@ Coverage Diff @@
## main #1391 +/- ##
=======================================
Coverage 95.61% 95.61%
=======================================
Files 15 15
Lines 776 776
=======================================
Hits 742 742
Misses 34 34
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Addresses electron/rebuild#1024
In node-gyp v9.0.0, node-gyp adds a python symlink for older versions of python. This symlink is added to a
${module_name}/build/node_gyp_bins/
directory within the build directory, which electron-rebuild then tries to rebuild.This PR ignores the
node_gyp_bins
directory, and adds tests for making sure that the directory is not added to the packaged app.