Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

Implement promises alongside callbacks #53

Merged

Conversation

vernak2539
Copy link
Contributor

  • promise integration (non-breaking)
  • added tests for all helper functions
  • renamed check401header to isValid401 (internal change, not breaking)
  • added fallback if respond is given invalid delivery type
  • clean up tests a little
  • removed all errorPropagatedFrom (people can figure it out)
=============================== Coverage summary ===============================
Statements   : 100% ( 101/101 )
Branches     : 97.78% ( 44/45 )
Functions    : 100% ( 14/14 )
Lines        : 100% ( 101/101 )
================================================================================

fixes #46

- promise integration (non-breaking)
- added tests for all helper functions
- renamed check401header to isValid401 (internal change, not breaking)
- added fallback if respond is given invalid delivery type
- clean up tests a little
- removed all errorPropagatedFrom (people can figure it out)
@vernak2539
Copy link
Contributor Author

@dougwilson mind taking a look when you have a sec? want to make sure i didn't mess anything up, or if there's a better way to do something

@dougwilson
Copy link
Contributor

My only initial comment is that it's weird that, when provided a callback, you still get a promise returned.

@vernak2539
Copy link
Contributor Author

Yeah thought that too, but it was the best way i thought to do it without it being overly complicated. doesn't seem to be too much overhead creating a promise even when using callbacks

@dougwilson
Copy link
Contributor

Gotcha. It's up to you, I was just sharing my opinion :) My concern wasn't performance-related, only what would happen when users did the following

RestClient
    .get(options, function (err, response) {
      // who gets called first, callback or promise?
    })
    .then(function(response) {
        // will be delivered with 200, 400, 401, 500, etc status codes
        // response.body === payload from response
        // response.res === full response from request client
        console.log(response);
    })
    .catch(function(err) {
        // error here
        console.log(err);
    });
}); 

@dougwilson
Copy link
Contributor

FWIW, I have been testing the waters myself on this kind of dual-API at https://github.com/stream-utils/raw-body and https://github.com/crypto-utils/uid-safe

@vernak2539
Copy link
Contributor Author

Oh I see what you're saying. I think it should be one or the other and people should implement it that way. I can add that to the readme.

Your stuff looks good at a glance, and probably implement something like that in v1.1 or something along those lines.

Appreciate the comments, as always

vernak2539 added a commit that referenced this pull request May 21, 2015
Implement promises alongside callbacks
@vernak2539 vernak2539 merged commit 58b8bd4 into salesforce-marketingcloud:master May 21, 2015
@vernak2539 vernak2539 deleted the promise-integration branch May 21, 2015 08:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable native promise usage
2 participants