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 FluxCollectToImmutableSet Refaster rule #571

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

mlrprananta
Copy link
Contributor

@mlrprananta mlrprananta commented Apr 7, 2023

Idea is based on an internal GH thread.

Suggested commit message:

Introduce `FluxCollectToImmutableSet` Refaster rule (#571)

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

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.

@mlrprananta mlrprananta requested review from rickie and Stephan202 April 7, 2023 10:11
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.

Thanks for the PR @mlrprananta 🚀 !

In our OSS repo we try to avoid linking to "internal" things or repositories, so updated the description :).

Made small tweaks to the suggested commit message 😄.

@rickie rickie added this to the 0.10.0 milestone Apr 7, 2023
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.

Added a commit to simplify a test 😬.

@@ -377,6 +377,10 @@ Flux<Integer> testFluxFilterSortWithComparator() {
return Flux.just(1, 4, 3, 2).sort(reverseOrder()).filter(i -> i % 2 == 0);
}

Mono<ImmutableSet<Integer>> testFluxCollectToImmutableSet() {
return Flux.just(1, 4, 3, 2).collect(toImmutableList()).map(ImmutableSet::copyOf);
Copy link
Member

Choose a reason for hiding this comment

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

On a second thought, let's simplify the numbers here. I think it makes sense to have 1, 2 as we are collecting and "usually" there will be multiple values to collect. The order used in the test above is important to the method that is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we also simplify for #570?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, will push a commit :).

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

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.

@Stephan202 Stephan202 force-pushed the mlrprananta/flux-collect-toimmutableset branch from 7e4550e to 28de12f Compare April 7, 2023 16:41
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.

Rebased and added a tiny tweak.

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

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
Copy link
Member

rickie commented Apr 7, 2023

I hesitated with the "more" as not all were really "more contrived" 😄.

@rickie rickie changed the title Introduce FluxCollectToImmutableSet refaster rule Introduce FluxCollectToImmutableSet Refaster rule Apr 7, 2023
@rickie rickie merged commit ae22e0e into master Apr 7, 2023
@rickie rickie deleted the mlrprananta/flux-collect-toimmutableset branch April 7, 2023 22:41
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