-
Notifications
You must be signed in to change notification settings - Fork 39
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
Introduce MonoFromFutureSupplier{,Boolean}
Refaster rules
#1244
Conversation
Looks good. No mutations were possible for these changes. |
Mono#fromFuture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Nice one @Venorcis :).
Suggested commit message:
Introduce `MonoFromFuture{,SuppressCancel}` Refaster rule (#1244)
(Commit message pending the rename comment ofcourse)
* Prefer {@link Mono#fromFuture(Supplier, boolean)} over {@link | ||
* Mono#fromFuture(CompletableFuture, boolean)}. | ||
*/ | ||
static final class MonoFromFutureSuppressCancel<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Stephan202 according to our undocumented naming scheme, we would now call this MonoFromFutureBoolean
right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking even MonoFromFutureSupplierBoolean
😄
3561c90
to
a005e37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased and added a commit. Updated suggested commit message:
Introduce `MonoFromFutureSupplier{,Boolean}` Refaster rules (#1244)
* Prefer {@link Mono#fromFuture(Supplier, boolean)} over {@link | ||
* Mono#fromFuture(CompletableFuture, boolean)}. | ||
*/ | ||
static final class MonoFromFutureSuppressCancel<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking even MonoFromFutureSupplierBoolean
😄
@BeforeTemplate | ||
Mono<T> before(CompletableFuture<T> future) { | ||
return Mono.fromFuture(future); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add an XXX
to later apply @NotMatches(IsIdentityOperation.class)
. Cause as-is this code will also rewrite Mono.fromFuture(someVariable)
, which isn't great.
Mono#fromFuture
MonoFromFutureSupplier{,Boolean}
Refaster rules
Looks good. No mutations were possible for these changes. |
Quality Gate passedIssues Measures |
Futures subscribe eagerly. This means that
Mono#fromFuture(CompletableFuture)
already executes the contents of the future on assembly time (even when thatFuture
is constructed from aMono
as we do in our reactive Caffeine caches).I therefore suggest instead to always use
Mono#fromFuture(Supplier)
, which will only instantiate (and thus execute) the future on subscription, i.e. much closer to how we usually do everything 😄