-
Notifications
You must be signed in to change notification settings - Fork 234
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
Easier rebuild of native modules #643
Conversation
electron/rebuild-native-modules.js
Outdated
const normalize = (args) => { | ||
return args.map((arg) => { | ||
Object.keys(process.env).forEach((key) => { | ||
const regex = new RegExp(`\\$${key}|%${key}%`, 'i'); |
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.
More descriptive name for regex
. What does it match?
electron/rebuild-native-modules.js
Outdated
const [command] = normalize(args); | ||
const proc = exec(command, (error, stdout, stderr) => { | ||
if (error) { | ||
console.error(`exec error: ${error}`); |
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.
It's always good to mention the original error object, like:
console.error(`exec error: ${error}`, error);
electron/rebuild-native-modules.js
Outdated
const [command] = normalize(args); | ||
const proc = exec(command, (error, stdout, stderr) => { | ||
if (error) { | ||
console.error('Execution error: ', error); |
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.
no need for colon space (:
) when using multi arguments
console.error('Execution error', error);
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.
True, no need for the space - but the colon makes the error message more readable.
* master: New native module rebuilding (wireapp#643) Publish amplify events when joining calls (wireapp#629) Use new V8 inspector (wireapp#641) electron 1.6.9 (wireapp#637) Release 2.14.2743 Add template for github issues Update libsodium-neon to 2.1.0 (wireapp#634) Update electron-build-env to 0.2.0 (wireapp#633)
wire_target_arch
.electron-rebuild
.