-
Notifications
You must be signed in to change notification settings - Fork 37
Conversation
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 for your work @jeremygooch. Don't worry about the review comments, I'll fix them myself. I have some time to work on this package right now so I want thing to move ahead quickly. I just added them so you get some feedbacks on what your contribution. 👍
@@ -141,6 +143,7 @@ fs.emptyDir(paths.appBuild) | |||
} | |||
|
|||
spinner.succeed(); | |||
runHook(afterRebuildHook); |
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 think it'd be better to use ora
to display a message before running the hooks so the user knows what is happening.
const exec = require('child_process').exec; | ||
|
||
const child = exec(hook, (error, stdout, stderr) => { | ||
console.log(`${stdout}`); |
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 don't think the template string is necessary here since stdout and stderr are already strings and you don't add anything to the string themselves.
|
||
const exec = require('child_process').exec; | ||
|
||
const child = exec(hook, (error, stdout, stderr) => { |
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.
We are not using the return from exec so no need to put in in variable IMO.
return; | ||
} | ||
|
||
const exec = require('child_process').exec; |
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 prefer to put the require
at the top of the file, just to be consistent with what has already been done.
* Adding cli option after-build-hook. * Specifying initial vs rebuild hooks.
This PR makes it possible to run a command after the initial build and rebuilds.