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

(fix) Re: Issue #624 (https://github.com/node-gh/gh/issues/624) #636

Closed keaglin closed 5 years ago

keaglin commented 5 years ago

gh re --new is working, tests passed. Current behavior: if a name is supplied, new remote repo will be given that name. If a name isn't supplied, it'll take the name of the pwd.

Also: I noted what Zev said about doing set-url but that seemed like a whole other thing (probably need child_process from node or a 3rd-party package).

protoEvangelion commented 5 years ago

Thanks for redoing this so quickly :) One thing I was curious about is how this syntax works:

options.repo, (options.new = getCurrentDirName())

Have never seen that before.

keaglin commented 5 years ago

That's a good question. I... now don't remember what I was accomplishing there. Let me look at the code a little later today and I'll get back to you on that one.

protoEvangelion commented 5 years ago

So all it does is assign the current folder name to options.new.

If that is what you meant to do then we can just have:

options.new = getCurrentDirName()
protoEvangelion commented 5 years ago

If that's the case let me know and I will make the change and merge 🎉

keaglin commented 5 years ago

Yeah I'm not sure what happened. It may have been something I forgot to clean up from an earlier version of this solution. But if taking off that random options.repo still works and passes the tests, I'm all for changing it!

protoEvangelion commented 5 years ago

hmm it's passing on everything except node v11.11 😢

 FAIL  __tests__/git.test.ts
  ● Test suite failed to run
    TypeError: Cannot assign to read only property 'Symbol(Symbol.toStringTag)' of object '#<process>'
      at exports.default (node_modules/jest-util/build/create_process_object.js:15:34)

I have never seen this error before. Can you take a look?

keaglin commented 5 years ago

Sure. Give me a moment

keaglin commented 5 years ago

I pulled down changes and rebuilt it but I can't seem to reproduce that. Everything is passing for me.

keaglin commented 5 years ago

Sorry to kind of "works on my machine" you there but I can't pinpoint what might be different.

protoEvangelion commented 5 years ago

:tada: This PR is included in version 1.17.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

protoEvangelion commented 5 years ago

Thanks for your quick work on this @keaglin! Much appreciated 💯

keaglin commented 5 years ago

Absolutely! Happy to help 😄