Issue #205 - Update to callback - Inquirer interface requires Promise

[zacharytelschow]

@jimthedev: I realize the irony of not using one of your commit adapters to submit a pull request, but I didn't see contribution guidelines in your README 😄 I have tested this change on my local and it works. Please let me know if there are any further changes you'd like to see.

[zacharytelschow]

Link to issue since it's actually in another repo.

[jmfrancois]

Please @jimthedev merge this.
This plugin just doesn't work at the moment, I have try the change and it works.

update: oh the tests... Could you @zacharytelschow update the tests ?

[jimthedev]

@janecakemaster originally wrote the tests, I wonder if she'd be willing to help. Also we need to get a working travis config.

[janecakemaster]

@jimthedev which test are we talking about? sry it's been a while since I've touched this 😁

[jimthedev]

@janecakemaster haha no worries. Basically @zacharytelschow made some changes in his fork that break the tests you previously wrote. He is afk for right now but if you run the following you'll see the tests that are failing:

git clone https://github.com/zacharytelschow/cz-jira-smart-commit.git
cd cz-jira-smart-commit
git checkout Issue205
npm install
npm run test

[LinusU]

I think that we should add inquirer as a dependency and require it ourselves, as discussed here: commitizen/cz-cli#249

This will prevent this from happening again :)

If we merge this as is, it will break for people using an older cz-cli (which they presumable are using since updating breaks...)

[jmfrancois]

But do not merge it means unusable for every new comer that just install last cz-cli (my case).

[jimthedev]

Yes, this would be a breaking change. We'd go to 2.0.

Jim ForCy

On July 22, 2016 at 5:03:03 AM, Jean-Michel FRANCOIS (
notifications@github.com) wrote:

But do not merge it means unusable for every new comer that just install
last cz-cli (my case).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGpiljNZ18AYZkmgaVFVPEgqjTloHKNks5qYJVXgaJpZM4Innzl
.

[janecakemaster]

@zacharytelschow sinon.spy doesn't support promises. you'll have to look into a test spy/stub utility that lets you stub a promise-y function. maybe https://www.npmjs.com/package/sinon-as-promised might work?

[LinusU]

This will not be a breaking change if we require and use our own version of inquirer

[bgannonPL]

I appreciate that you all care about fixing this the right way. But in the meantime, people depend on this package and it has been unusable for some time with the current version of commitizen.

[LinusU]

@bgannonPL the correct fix is here 🎉 -> #5

[LinusU]

Resolved with #5