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

Add Refaster rule for Flux#concat for a single Mono or Flux #20

Merged
merged 3 commits into from
Nov 13, 2021

Conversation

werli
Copy link
Member

@werli werli commented Nov 10, 2021

Thx to @rickie for contributing 👍

@werli werli requested a review from rickie November 10, 2021 07:39
Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

I pushed some suggestions, PTAL :). Nice template Phil 🚀

@@ -161,6 +161,32 @@ private ReactorTemplates() {}
}
}

/** Don't use {@link Flux#concat(Publisher)} with only a single {@link Mono}. */
abstract static class FluxConcatOfOneMono<T> {
Copy link
Member

Choose a reason for hiding this comment

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

We can use static final here instead of abstract. abstract for templates is only when we need to use the Refaster @Placeholder.

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, the convention for naming the templates is almost exactly describing the beforetemplate content. So it seems that we can even drop OfOne.
I'll push something :)

Copy link
Member Author

Choose a reason for hiding this comment

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

🧑‍🎓, thanks @rickie !

Copy link
Member

Choose a reason for hiding this comment

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

I don't know why I described it precisely the other way around here.. My bad 😅

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 :)

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.

Tnx!

Comment on lines +177 to +188
/** Don't unnecessarily invoke {@link Flux#concat(Publisher)}. */
static final class FluxIdentity<T> {
@BeforeTemplate
Flux<T> before(Flux<T> flux) {
return Flux.concat(flux);
}

@AfterTemplate
Flux<T> after(Flux<T> flux) {
return flux;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

@rickie This template could be folded into the "vacuous conversion" bug checker we discussed a while back.

Copy link
Member

Choose a reason for hiding this comment

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

Nice! Will add it to the list @Stephan202

@Stephan202 Stephan202 merged commit 17e74f7 into master Nov 13, 2021
@Stephan202 Stephan202 deleted the werli/reactor-flux-concat branch November 13, 2021 17:04
@Stephan202 Stephan202 added this to the 0.1.0 milestone Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants