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

Introduce FluxFromStreamSupplier Refaster rule #1261

Merged
merged 2 commits into from
Aug 7, 2024

Conversation

rickie
Copy link
Member

@rickie rickie commented Jul 24, 2024

No description provided.

@rickie rickie added this to the 0.18.0 milestone Jul 24, 2024
@rickie rickie requested review from Stephan202 and EnricSala July 24, 2024 09:27
Copy link

@EnricSala EnricSala left a comment

Choose a reason for hiding this comment

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

Thanks for picking up 💪

* may defer initiation of the asynchronous computation until subscription.
*/
static final class FluxFromStreamSupplier<T> {
@BeforeTemplate

Choose a reason for hiding this comment

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

Suggested change
@BeforeTemplate
// XXX: Constrain the `stream` parameter using `@NotMatches(IsIdentityOperation.class)` once
// `IsIdentityOperation` no longer matches nullary method invocations.
@BeforeTemplate

Is this XXX also relevant in this case?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is 👍

Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Added a commit. Suggested commit message:

Introduce `FluxFromStreamSupplier` Refaster rule (#1261)

@@ -641,4 +643,8 @@ Mono<Void> testMonoFromFutureSupplier() {
Mono<Void> testMonoFromFutureSupplierBoolean() {
return Mono.fromFuture(() -> CompletableFuture.completedFuture(null), true);
}

Flux<Integer> testFluxFromStreamSupplier() {
return Flux.fromStream(() -> RandomGenerator.StreamableGenerator.of(1));
Copy link
Member

Choose a reason for hiding this comment

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

👀

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn, I'm not sure what I was doing here 😬.

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 think something with auto-complete I'd say, but slipped through.

* may defer initiation of the asynchronous computation until subscription.
*/
static final class FluxFromStreamSupplier<T> {
@BeforeTemplate
Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is 👍

@@ -1913,7 +1914,7 @@ Duration after(StepVerifier.LastStep step, Duration duration) {

/**
* Prefer {@link Mono#fromFuture(Supplier)} over {@link Mono#fromFuture(CompletableFuture)}, as
* the former may defer initiation of the asynchornous computation until subscription.
* the former may defer initiation of the asynchronous computation until subscription.
Copy link
Member

Choose a reason for hiding this comment

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

Nice one; should also be fixed below.

static final class FluxFromStreamSupplier<T> {
@BeforeTemplate
Flux<T> before(Stream<T> stream) {
return Flux.fromStream(stream);
Copy link
Member

Choose a reason for hiding this comment

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

This triggers an interaction with FluxFromIterable, so more changes are necessary; will apply.

Comment on lines 1952 to 1963
/**
* Prefer {@link Flux#fromStream(Supplier)} over {@link Flux#fromStream(Stream)}, as the former
* may defer initiation of the asynchronous computation until subscription.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Not related to asynchronicity 👀.

Suggested change
/**
* Prefer {@link Flux#fromStream(Supplier)} over {@link Flux#fromStream(Stream)}, as the former
* may defer initiation of the asynchronous computation until subscription.
*/
/**
* Prefer {@link Flux#fromStream(Supplier)} over {@link Flux#fromStream(Stream)}, as the former
* yields a {@link Flux} that is more likely to behave as expected when subscribed to more than
* once.
*/

Copy link

github-actions bot commented Aug 4, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@rickie rickie force-pushed the rossendrijver/flux_from_stream branch from 825d644 to 0eedcae Compare August 7, 2024 08:32
Copy link

github-actions bot commented Aug 7, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

sonarqubecloud bot commented Aug 7, 2024

@rickie rickie merged commit 77d183f into master Aug 7, 2024
15 checks passed
@rickie rickie deleted the rossendrijver/flux_from_stream branch August 7, 2024 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants