-
Notifications
You must be signed in to change notification settings - Fork 51
update to es6 promises and remove when dependency #267
Conversation
3562c8f
to
97e330b
Compare
That looks like something we want. What do you think @kumar303? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems fine but I haven't had a chance to test it.
response.responseBody); | ||
|
||
if (response.responseError instanceof Error) { | ||
throw response.responseError; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this still work the same as before? I'm not sure what was going on with the callbacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, do tests that rely on error responses still get the same behavior? To research it, you could try and make one of the tests fail that was relying on this mock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ccarruitero this would need to be rebased on master to pick up the new |
97e330b
to
42c3fd1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again, this looks great. I tested the MockRequest
changes and everything works as expected.
No description provided.