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

refactor(Subscription): return always subscription when calling Subscription.add() #1840

Merged

Conversation

tetsuharuohzeki
Copy link
Contributor

@tetsuharuohzeki tetsuharuohzeki commented Jul 19, 2016

BREAKING CHANGE:

This fixes #1656.
This makes Subscription.add() returns always subscription.
if you checks the returned value of it. you might need to change it.

@kwonoj
Copy link
Member

kwonoj commented Jul 19, 2016

sorry for nitpicking - doesn't breaking need to be in commit message footer? (https://github.com/ReactiveX/rxjs/blob/master/CONTRIBUTING.md#footer)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.701% when pulling 4dab77c on saneyuki:always-return-subscription into d6dcfe1 on ReactiveX:master.

@tetsuharuohzeki
Copy link
Contributor Author

@kwonoj I fixed the message.

@tetsuharuohzeki tetsuharuohzeki changed the title refactor(Subscription): BREAKING return always subscription when calling Subscription.add() refactor(Subscription): return always subscription when calling Subscription.add() Jul 19, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.702% when pulling 6fb7c42 on saneyuki:always-return-subscription into d6dcfe1 on ReactiveX:master.

@tetsuharuohzeki
Copy link
Contributor Author

@kwonoj

rebased!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.702% when pulling 5f280ed on saneyuki:always-return-subscription into 5760d5a on ReactiveX:master.

it('Should returns the passed one if passed a AnonymousSubscription having not function `unsubscribe` member', () => {
const sub = new Subscription();
const arg = {
isUnsubscribed: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops. this should be false for testing correctly

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.702% when pulling 94ce8ac on saneyuki:always-return-subscription into 5760d5a on ReactiveX:master.

@tetsuharuohzeki
Copy link
Contributor Author

@kwonoj I rebased this on the latest master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.708% when pulling 54b40b6 on saneyuki:always-return-subscription into 9b8afb8 on ReactiveX:master.

@jayphelps
Copy link
Member

LGTM, one more rebase and I'll merge!

…ription.add()

BREAKING CHANGE:

This fixes ReactiveX#1656.
This makes Subscription.add() returns always subscription.
if you checks the returned value of it. you might need to change it.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 96.723% when pulling 03f9d0c on saneyuki:always-return-subscription into 27a88d2 on ReactiveX:master.

@jayphelps jayphelps merged commit 897fe3b into ReactiveX:master Jul 31, 2016
@jayphelps
Copy link
Member

Thank you! 💃

@tetsuharuohzeki tetsuharuohzeki deleted the always-return-subscription branch July 31, 2016 08:34
@tetsuharuohzeki
Copy link
Contributor Author

you're welcome!

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
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.

Subscription.add should return the subscription.
4 participants