Skip to content

Improve Mono fromFuture/fromCompletionStage javadocs#3272

Merged
simonbasle merged 2 commits into3.4.xfrom
monoFromFutureNotCompletableFuture
Nov 4, 2022
Merged

Improve Mono fromFuture/fromCompletionStage javadocs#3272
simonbasle merged 2 commits into3.4.xfrom
monoFromFutureNotCompletableFuture

Conversation

@simonbasle
Copy link
Copy Markdown
Contributor

@simonbasle simonbasle commented Nov 3, 2022

This commit revises javadocs of Mono fromFuture and fromCompletionStage
methods to better reflect the cancellation behavior.

Fixes #3252.

@simonbasle simonbasle added this to the 3.4.25 milestone Nov 3, 2022
@simonbasle simonbasle added type/documentation A documentation update type/enhancement A general enhancement labels Nov 3, 2022
@simonbasle simonbasle self-assigned this Nov 3, 2022
@simonbasle simonbasle requested a review from a team November 3, 2022 17:38
@simonbasle
Copy link
Copy Markdown
Contributor Author

this is draft as I'd like to add a bit of test coverage with a CompletionStage+Future that isn't a CompletableFuture. @reactor/core-team please review the prod side of the change nonetheless

@simonbasle
Copy link
Copy Markdown
Contributor Author

relates to #3252

could be useful to Reactor-Netty for Netty5 as they have to wrap a Netty5 future that is CompletionStage and Future but not CompletableFuture.

@simonbasle simonbasle requested review from a team and rstoyanchev November 3, 2022 17:43
@simonbasle
Copy link
Copy Markdown
Contributor Author

fixes #3235

Copy link
Copy Markdown
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

After this + recent changes, there are 2 fromCompletionStage and 5 fromFuture methods, which is a bit much. It seems to me the changes here evolve the original API, and effectively supersede the original fromCompletionStage methods + the fromFutureCompletableFuture, so maybe those could be deprecated accordingly so that the count eventually comes down to 4?

@simonbasle
Copy link
Copy Markdown
Contributor Author

this isn't so great anyway, because with widened types the Supplier variants become ambiguous for the compiler :(

Ambiguous method call. Both
fromFuture (Future<?> & CompletionStage<?>, boolean)
in Mono and
fromFuture (Supplier<CombinedFuture>, boolean) in Mono match

I will refocus this PR to be a documentation improvement only (as per the linked issue)

@simonbasle simonbasle changed the title Widen the type of Mono.fromFuture, improve javadoc Improve Mono fromFuture/fromCompletionStage javadocs Nov 4, 2022
@simonbasle simonbasle marked this pull request as ready for review November 4, 2022 15:28
@simonbasle simonbasle removed the type/enhancement A general enhancement label Nov 4, 2022
This commit widens the type of `Mono.fromFuture` to accept any class
which is both a `CompletionStage` and a `Future`.

In the case of `fromFuture(CompletableFuture)`, the original signature
is kept for binary compatibility but an overload with the intersection
type is also provided.

In the case of `Supplier` based overloads and of methods that were just
introduced in the current snapshot, CompletableFuture type is replaced
with the intersection type.

Javadocs of all these methods as well as the fromCompletionStage methods
has been revised to better reflect this and hint at the cancellation
behavior.
@simonbasle simonbasle force-pushed the monoFromFutureNotCompletableFuture branch from 834943f to 6db309d Compare November 4, 2022 16:54
@simonbasle simonbasle merged commit e8406cd into 3.4.x Nov 4, 2022
@simonbasle simonbasle deleted the monoFromFutureNotCompletableFuture branch November 4, 2022 16:55
@reactorbot
Copy link
Copy Markdown

@simonbasle this PR seems to have been merged on a maintenance branch, please ensure the change is merge-forwarded to intermediate maintenance branches and up to main 🙇

simonbasle added a commit that referenced this pull request Nov 4, 2022
chemicL pushed a commit that referenced this pull request Mar 7, 2023
This commit revises javadocs of Mono fromFuture and fromCompletionStage
methods to better reflect the cancellation behavior.

Fixes #3252.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation A documentation update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants