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

Add SHXPromise to implementations #142

Merged
merged 1 commit into from
Aug 29, 2013

Conversation

MSNexploder
Copy link
Contributor

No description provided.

@briancavalier
Copy link
Member

I have almost no experience with Objective-C, but the code looks clean, and has tests :)

@MSNexploder: Does it ensure that onFulfilled and onRejected are invoked asynchronously? Does such a mechanism exist in Objective-C?

It looks like in handlePromiseWithValue handles the case of value being a promise (but again, I barely parse Obj-C, so I could be totally off!), and uses the term fulfill in either case. Typically, that polymorphic operation of handling either a promise or a value is called resolve. It's not a problem--especially since we don't have a spec for it--just fyi.

@MSNexploder
Copy link
Contributor Author

I tried to keep the tests as close to the original Promises/A+ compliance test suite as possible.

All callbacks are invoked using dispatch_async which basically defers the execution to a later time (+ optionally to a different execution queue / thread).

Actually, you're right about fulfill vs resolve. No idea why I didn't keep the original naming...

@briancavalier
Copy link
Member

I tried to keep the tests as close to the original Promises/A+ compliance test suite as possible.

Awesome.

All callbacks are invoked using dispatch_async

Ah, I missed that. Thanks!

Looks good to me. @domenic?

@domenic
Copy link
Member

domenic commented Aug 29, 2013

Looks great to me!

briancavalier added a commit that referenced this pull request Aug 29, 2013
Add SHXPromise to implementations
@briancavalier briancavalier merged commit cb472c7 into promises-aplus:master Aug 29, 2013
@MSNexploder MSNexploder deleted the patch-1 branch August 29, 2013 16:29
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.

3 participants