node-gh / gh

(DEPRECATED) GitHub CLI made with NodeJS. Use the official https://cli.github.com/ instead.
http://nodegh.io
Other
1.71k stars 217 forks source link

Fixes errors happening when '--no-hooks' was given. #533

Closed Jeffrey-Zutt closed 6 years ago

Jeffrey-Zutt commented 6 years ago

First of all, let me start by saying great job on Node GH! It's a real lifesaver for me and my team!

I've noticed a small bug though: When the --no-hooks flag is given, Node-GH sometimes resolves in an error.

For example: gh pr 1 --no-hooks --fetch --merge --branch my-branch

Will result in:

TypeError: afterHooksCallback is not a function
    at /Users/jeffrey/.nvm/versions/node/v8.9.4/lib/node_modules/gh/lib/cmds/pull-request.js:868:13
    at /Users/jeffrey/.nvm/versions/node/v8.9.4/lib/node_modules/gh/lib/cmds/pull-request.js:346:9
    at /Users/jeffrey/.nvm/versions/node/v8.9.4/lib/node_modules/gh/node_modules/github/api/v3.0.0/pullRequests.js:118:17
    at callCallback (/Users/jeffrey/.nvm/versions/node/v8.9.4/lib/node_modules/gh/node_modules/github/index.js:743:17)
    at IncomingMessage.<anonymous> (/Users/jeffrey/.nvm/versions/node/v8.9.4/lib/node_modules/gh/node_modules/github/index.js:796:25)
    at emitNone (events.js:111:20)
    at IncomingMessage.emit (events.js:208:7)
    at endReadableNT (_stream_readable.js:1055:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)

I have simply passed a lodash.noop on the opt_callback() in lib/hooks.js to fix this issue. I think it's good solution because opt_callback always expects a callback anyway.

Let me know if you want me to make any tweaks, I will see if I have time to implement them!

protoEvangelion commented 6 years ago

@Jeffrey-Zutt Thanks for opening up a PR on this! Taking a look now :grinning: