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

Allow promises to have multiple then/rescue/always #1263

Merged
merged 2 commits into from
Jan 8, 2016

Conversation

jgaskins
Copy link
Contributor

According to the A+ promise specification, "then may be called multiple times on the same promise". Since fail and always are just then with different success/failure semantics, I added the same functionality to them, as well.

@wied03, @meh, and I spoke about supporting this in the past. Last I heard, @meh was still contemplating the change. I probably should've put this in before the 0.9 release, but this PR is backwards-compatible with the current release.

@elia
Copy link
Member

elia commented Dec 27, 2015

👍

Master is already targeting 0.10 but if someone needs it individual commits/PRs can be backported to 0.9 and released in 0.9.1

@meh
Copy link
Member

meh commented Dec 27, 2015

If it works I'm fine with it, I just have one request, can you add then! and friends that check for presence of an existing next?

According to the A+ promise specification, "then may be called multiple
times on the same promise" -- https://promisesaplus.com/#point-36
@jgaskins
Copy link
Contributor Author

I'm not sure what you mean. You want one that does raise if called multiple times on the same promise?

@meh
Copy link
Member

meh commented Dec 27, 2015

Yes, alternative bang methods that raise.

@jgaskins
Copy link
Contributor Author

What was the reason behind the single-use method again? I've asked this before but I don't remember.

@meh
Copy link
Member

meh commented Dec 27, 2015

I just personally prefer it that way to avoid bugs in the code when you only want to chain and not add up.

@jgaskins
Copy link
Contributor Author

Alrighty, done, but it looks like GitHub diffs are broken at the moment?

@meh
Copy link
Member

meh commented Dec 27, 2015

I'll merge after they're back.

meh added a commit that referenced this pull request Jan 8, 2016
Allow promises to have multiple then/rescue/always
@meh meh merged commit 79415d1 into opal:master Jan 8, 2016
@jgaskins jgaskins deleted the promise-multiple-thens branch October 30, 2016 03:27
hmdne pushed a commit to hmdne/opal that referenced this pull request Jan 27, 2024
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