Implement Promise API

[bradberger]

Per issue #15 add a new promise aware api. If a global Promise constructor is available, then it will return a promise, if not the api returns undefined. The old callback api should always be supported.

[denheck]

Hey @bradberger, I would like to give adding this feature a try. I have a question for you before I get started.

It looks like prompt, confirm and alert all have a fluent interface and return "this". How would you like to determine when to keep the interface fluent and when to return a Promise. I want to avoid breaking backward compatibility as much as possible.

One potential solution is call a method to enable promises similar to closeLogOnClick.
Example:

var promise = alertify.enablePromises(true).prompt("This is a prompt dialog");

Please let me know what you think and thanks for resurrecting Alertify!

[bradberger]

@denheck Thanks for being willing to help out!

I actually don't think it would be an issue to just not return this and return the promise instead to keep things simple. That the interface is fluent for the actual methods like log() etc isn't actually anything that's published anywhere as far as I can remember, so using it that was isn't really officially a feature. Nor do any unit tests check for that.

That's just my thought, but I'm not 100% sure how other people are using it, so would also be interested to hear if anyone else has anything to add.

[denheck]

@bradberger here is a link to my PR: #47

Let me know if you would like me to change anything. Thanks!

[bradberger]

Looks great, thanks! I'll double check everything tomorrow and merge it.

The only thing is if it's possible to add tests for it, that would be great. So far the project is kind of behind the whole testing game and has very low coverage. but if we can implement it for new changes that would be great.

[bradberger]

@denheck Also sent you an invitation to join the contributors team, we're definitely looking to get more people on board to help out with the project and try to keep it alive 😄

[denheck]

@bradberger Thanks for the invite! I would be happy to add tests for it. I should be able to do that over the weekend.

[denheck]

I am having a hard time getting Sauce Connect to start when I run npm test. I think I may need to specify a username and password. Is there a separate dotfile with credentials that I need or something similar to get the test suite to run on my machine?

Karma Debug Log:

09 10 2015 09:35:38.795:DEBUG [launcher.sauce]: Sauce Connect is already running or starting
09 10 2015 09:35:38.796:DEBUG [launcher.sauce]: Opening local tunnel using Sauce Connect
09 10 2015 09:35:38.798:DEBUG [launcher.sauce]: Starting sc with args: --tunnel-identifier karma1444401339 --readyfile /var/folders/zh/9dqly4bx2fqdb8j36114bc6r0000gn/T/sc-launcher-readyfile
09 10 2015 09:35:38.811:DEBUG [launcher.sauce]: Could not start Sauce Connect. Exit code 1 signal: null

Here is the output I get when running Sauce Connect manually:

$ ./node_modules/karma-sauce-launcher/node_modules/sauce-connect-launcher/sc/sc-4.3.8-osx/bin/sc --tunnel-identifier karma1444401339 --readyfile /var/folders/zh/9dqly4bx2fqdb8j36114bc6r0000gn/T/sc-launcher-readyfile
09 Oct 09:40:50 - Sauce Connect 4.3.8, build 1674 aba74e5
09 Oct 09:40:50 - Error, no user specified.

[bradberger]

Yeah, I think that has to be set up. I'll try to get you that when I get back in.

I actually think the workflow for local development doesn't even require it, as the there are gulp commands for local testing, and that's more of a CI test, but either way let me see what I can do to get you those credentials when I get back in...

Just sent you an invite for the CI system too, so you can re-run tests, etc, there as needed, too.

[bradberger]

Actually think I'm going to need to create a Alertify team on Sauce, so will try to get to that later on this evening, then you can set up your own credentials as a team member.

[denheck]

Thanks @bradberger, that helped solve my problem! I forgot to commit the dist files and quickly realized it after running the ci tests on my local repo. I updated my PR and the tests are now passing. Let me know if you need me to make any other changes before merging.

[bradberger]

Awesome, thanks @denheck! It's been a crazy day and haven't had a chance to look at it yet, but I'm sure it's all good 😄

[bradberger]

Just updated bower and npm, too, so everything's merged. The website should be too, though it might take a bit for the cache to show the changes.

@denheck Thanks again!