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

Node GH will fail if the latest commit message is empty and title is not specified as an argument #647

Closed cleydyr closed 4 years ago

protoEvangelion commented 5 years ago

Hi @cleydyr in what case would a commit message every be empty?

cleydyr commented 5 years ago

Sorry for not posting the steps to reproduce.

I ended up with a empty commit message accidentally somehow, but there's a deliberate way of doing that, like in git commit --allow-empty-message -m ''.

Thanks

protoEvangelion commented 5 years ago

Does it fail on every command? Do you have the error message?

cleydyr commented 5 years ago

Sure. Here are the steps to reproduce.

  1. Create and checkout a test branch on a repository with a valid remote on Github;
  2. Create a dummy file just for test purposes;
  3. Stage the previously created file;
  4. Commit the staged changes with git commit --allow-empty-message -m ''
  5. Submit a pull request with gh pr --submit mygithubuser, replacing mygithubuser with your user name from Github.

The following error will be seen in the terminal

undefined:1
Empty value for parameter 'title':
^

SyntaxError: Unexpected token E in JSON at position 0
    at JSON.parse (<anonymous>)
    at instance.submit (/Users/thecleydyr/.nvm/versions/node/v8.11.1/lib/node_modules/gh/lib/cmds/pull-request.js:985:32)
    at base.github.pullRequests.getAll (/Users/thecleydyr/.nvm/versions/node/v8.11.1/lib/node_modules/gh/lib/cmds/pull-request.js:290:25)
    at /Users/thecleydyr/.nvm/versions/node/v8.11.1/lib/node_modules/gh/node_modules/github/api/v3.0.0/pullRequests.js:71:17
    at callCallback (/Users/thecleydyr/.nvm/versions/node/v8.11.1/lib/node_modules/gh/node_modules/github/index.js:743:17)
    at IncomingMessage.<anonymous> (/Users/thecleydyr/.nvm/versions/node/v8.11.1/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:1064:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)

Thanks

protoEvangelion commented 5 years ago

@all-contributors please add cleydyr for bug

allcontributors[bot] commented 5 years ago

@protoEvangelion

I've put up a pull request to add @cleydyr! :tada:

protoEvangelion commented 4 years ago

Fixed by: https://github.com/node-gh/gh/pull/716