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

2.x: improve autoConnect() Javadoc + add its marble #5756

Merged
merged 1 commit into from
Dec 7, 2017

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Dec 6, 2017

This PR improves the JavaDoc of ConnectableObservable.autoConnect() and ConnectableFlowable.autoConnect() operators and adds the respective marble diagrams:

autoConnect

autoConnect

@akarnokd akarnokd added this to the 2.2 milestone Dec 6, 2017
* <p>
* The connection happens after the first subscription and happens at most once
* during the lifetime of the returned Flowable. If this ConnectableFlowable
* terminates, the connection is never renewed, no matter how Subscribers come
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe no matter how many Subscribers

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't feel "many" should be there.

Copy link
Contributor

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

I liked it 👍

* <p>
* This overload does not allow disconnecting the connection established via
* {@link #connect(Consumer)}. Use the {@link #autoConnect(int, Consumer)} overload
* to gain access to the {@code Disposable} representing the only connection.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add @see refCount() and another overload?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same below + in ConnectableObservable

Copy link
Member Author

Choose a reason for hiding this comment

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

I omitted refCount here because there is no refCount(int) yet and that would confuse readers.

@akarnokd akarnokd merged commit ec40a5e into ReactiveX:2.x Dec 7, 2017
@akarnokd akarnokd deleted the AutoConnectJavadoc branch December 7, 2017 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants