Add preinstall script #4
Conversation
|
@gabru-md Could you test this. |
ghost
mentioned this pull request
|
preinstall.js ? is it missing ? |
|
I've added the file. See 6ff481b. I don't know how it never got commited. I usually run |
preinstall.js
Outdated
| var spawn = require('child_process').spawn; | ||
| var readline = require('readline'); | ||
| var terminal = require('terminal-utilities') | ||
|
|
There was a problem hiding this comment.
chalk is missing from the modules.
preinstall.js
Outdated
| }); | ||
|
|
||
| exec('which wget', function(err,stdout,stderr){ | ||
| if(err){ |
There was a problem hiding this comment.
there is a problem with the use of if/else here all throughout the code.
if an error is encountered it means that wget is not present, and hence must be installed. But the code seems to do something else.
preinstall.js
Outdated
| if(stdout){ | ||
| console.log('wget is required to install this module'); | ||
| exec('which apt-get', function(err,stdout,stderr){ | ||
| if(stdout){ |
preinstall.js
Outdated
| terminal.write(data); | ||
| }); | ||
| installation.on('close', function(code){ | ||
| console.log('wget installed successfully'); |
There was a problem hiding this comment.
process.exit(0) is missing.
the process stops thereafter and has to be manually terminated.
preinstall.js
Outdated
| } | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
some brackets are missing. fix them as well.
:D
|
@gabru-md I've made the changes that you have requested. |
|
there were still some issues to be resolved. due to asynchronous nature of Node.JS, the Thanks for the contribution though. |
|
@gabru-md The reason I set it to be |
I haven't tested this because uninstalling wget uninstalls google-chrome for some reason.