Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promises api #17

Merged
merged 8 commits into from
Apr 4, 2016
Merged

Promises api #17

merged 8 commits into from
Apr 4, 2016

Conversation

edulan
Copy link
Contributor

@edulan edulan commented Jan 11, 2016

What

Use ES6 Promises instead of callbacks so we can easily chain actions and handle errors.

Usage:

var autoLaunch = new AutoLaunch({name: 'AwesomeApp'});

autoLaunch.enable().then(function() {
  // Do stuff when enabled
});

autoLaunch.disable().then(function() {
  // Do stuff when disabled
});

autoLaunch.isEnabled().then(function(enabled) {
  if (enabled) {
    // Do something if enabled
  } else {
    // Do something if disabled
  }
});

// Detect when something is not working
autoLaunch.disable().catch(function(error) {
  // Handle error
});

Concerns

Only compatible with node versions >= 11.13.

@andresgutgon
Copy link

🍏 👍


mkdirp.sync(@getDir())
fs.writeFile file, data, (err) ->
return reject if err?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be best to pass the error when calling reject.

@4ver
Copy link
Member

4ver commented Jan 11, 2016

Am all for this 👍 If you could get the tests working again that would be great.

Thanks for your contribution! 💃

This was referenced Jan 11, 2016
@edulan
Copy link
Contributor Author

edulan commented Jan 13, 2016

@4ver I've added TODO reminders and fixed some error stuff for Linux adapter. I agree with you to separate error handling to another PR 👍.

Also keep in mind Promises support is for node versions >= 11.13. Don't know if you want to create a special branch/version due to incompatibility issues with previous releases. Other option would be to add a polyfill package, but IMHO I'd keep the library as simple and dependency-free as possible.

Thank you for your consideration 😄

@4ver
Copy link
Member

4ver commented Jan 20, 2016

Sorry for the delay in getting back to you. I think we should go ahead and just use https://github.com/jakearchibald/es6-promise. It's pretty lightweight so shouldn't be much of an issue.

Thanks @edulan

@edulan
Copy link
Contributor Author

edulan commented Jan 21, 2016

Cool! I'll include es6-promise then 👍

@4ver 4ver merged commit 2c7e84c into Teamwork:master Apr 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants