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

Deprecated Flux.subscribe method used in documentation #3090

Closed
jdsdc opened this issue Jun 23, 2022 · 3 comments
Closed

Deprecated Flux.subscribe method used in documentation #3090

jdsdc opened this issue Jun 23, 2022 · 3 comments
Labels
good first issue Ideal for a new contributor, we'll help type/documentation A documentation update
Milestone

Comments

@jdsdc
Copy link

jdsdc commented Jun 23, 2022

Documentation Issue

Deprecated method subscribe method taking a subscriber as method parameter is used in example.
Version: 3.4.19
Chapter: 4.3.1.

Flux.subscribe(consumer, errorConsumer, completeConsumer, subscriptionConsumer)

Flux<Integer> ints = Flux.range(1, 4);
ints.subscribe(i -> System.out.println(i),
    error -> System.err.println("Error " + error),
    () -> System.out.println("Done"),
    sub -> sub.request(10));

Suggestion

BaseSubscriber<Integer> baseSubscriber = new BaseSubscriber<Integer>() {
            @Override
            protected void hookOnSubscribe(Subscription subscription) {
                subscription.request(2);
            }
            @Override
            protected void hookOnNext(Integer value) {
                System.out.println(value);
            }
            @Override
            protected void hookOnComplete() {
                System.out.println("Completed");
            }
        };
        Flux.range(1, 4)
                .subscribe(baseSubscriber);

@reactorbot reactorbot added the ❓need-triage This issue needs triage, hasn't been looked at by a team member yet label Jun 23, 2022
@jdsdc
Copy link
Author

jdsdc commented Jun 23, 2022

Missed that there was a chapter below describing BaseSubsciber (4.3.3). But anyhow, usage of deprecated methods maybe shouldn't be included in examples.

@simonbasle simonbasle added good first issue Ideal for a new contributor, we'll help type/documentation A documentation update and removed ❓need-triage This issue needs triage, hasn't been looked at by a team member yet labels Jun 29, 2022
@simonbasle simonbasle added this to the 3.4.x Backlog milestone Jun 29, 2022
@simonbasle
Copy link
Member

for reference, there is currently a NOTE admonition for that particular paragraph / method overload that states:

NOTE: That variant requires you to do something with the Subscription (perform a
request(long) on it or cancel() it). Otherwise the Flux hangs.

@jdsdc would it improve the situation if the NOTE was turned into a WARNING and have a few words about how this method is discouraged and to link to the BaseSubscriber section, or do you feel this should be removed entirely from the guide?

@jdsdc
Copy link
Author

jdsdc commented Jun 30, 2022

@simonbasle, I think the documentation should reflect the latest version and not use deprecated versions since documentation for the deprecated methods can be found in the documentation for earlier releases.

simonbasle added a commit that referenced this issue Aug 8, 2022
This commit removes the mention of the subscribe variant that takes a
Consumer<Subscription> from the reference guide.
This method is deprecated in order to discourage it, as it is easy to
misuse it and cause hanging when no request is explicitly made by
the Consumer.

Fixes #3090.
simonbasle added a commit that referenced this issue Aug 8, 2022
This commit removes the mention of the subscribe variant that takes a
Consumer<Subscription> from the reference guide.
This method is deprecated in order to discourage it, as it is easy to
misuse it and cause hanging when no request is explicitly made by
the Consumer.

Fixes #3090.
@OlegDokuka OlegDokuka modified the milestones: 3.4.x Backlog, 3.4.22 Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Ideal for a new contributor, we'll help type/documentation A documentation update
Projects
None yet
Development

No branches or pull requests

4 participants