fix: exit process on success to continue execution of other scripts

[spirosikmd]

No description provided.

[kentcdodds]

LGTM

[jfmengels]

I'm fine with adding this, but I don't see what behavior this fixes, could you explain it a bit more?

[spirosikmd]

Sure @jfmengels! I noticed the problem when I was calling the all-contributors add <username> <contributions> --commit command using child-process-promise. When the command was failing I was getting exit code 1 which would then .catch. However, when the command was successful, I was not getting any feedback, and the process would halt, subsequent .then would not execute.

What do you think? Is this a proper fix? Or is it something else I am missing?

[jfmengels]

And adding this line of code fixes that? That sounds odd, as the command terminates (I assume with code 0), so that sounds more like a bug in that library, or something they should document how to deal with better. If you don't mind taking another look around (maybe play with calling really simple node.js executables etc) and see if this fix is really necessary.

FYI, should this be actually necessary, then you'd need to add process.exit calls to other parts. These function calls will not call process.exit() at the moment unless an error is created somewhere.

[spirosikmd]

Thank you for the response and the recommendation @jfmengels! I tried the following commands. When I run this (and other similar git commands)

spawn('git', ['log'])
  .then((resp) => {
    console.log('RESP', resp);
  });

the then is executed successfully. When I run this

spawn('./node_modules/.bin/all-contributors', [
  'add', 'spirosikmd', 'code',
])
  .then((resp) => {
    console.log('RESP', resp);
  });

the then is never executed. I also put a console.log command in cli.js to check the process exit code

function onError(error) {
  if (error) {
    console.error(error.message);
    process.exit(1);
  }
  console.log(process.exitCode);
}

and it was actually undefined. This onError function is called every time, either on success or on error, it is the general callback, right? So I guess process.exit(0) in this place makes sense? Also I don't think the process.exit() needs to be added anywhere else, as I checked that onError is called every time for every command either init, generate, or add.

[jfmengels]

Ok, this is odd but I'm fine with it.
(it may be something worth mentioning in the child-process-promise repo though, to make the exit code 0 by default)

Thanks @spirosikmd :)

[jfmengels]

Automatically released thanks to @kentcdodds' scripts 😄