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

Fix several RxJava2Adapter Refaster templates #205

Merged
merged 1 commit into from
Aug 22, 2022

Conversation

Stephan202
Copy link
Member

@Stephan202 Stephan202 commented Aug 19, 2022

Suggested commit message:

Fix several `RxJava2Adapter` Refaster templates (#205)

This reverts some changes from d2bbee3ed9953598d3e75ff5bd381a2616b8693d
and instead drops some invalid `Flowable#compose` and `Flux#transform`
rewrite rules.

Commit referenced is d2bbee3.

This reverts some changes from d2bbee3
and instead drops some invalid `Flowable#compose` and `Flux#transform`
rewrite rules.
@Stephan202 Stephan202 added this to the 0.1.1 milestone Aug 19, 2022
@Stephan202 Stephan202 requested a review from rickie August 19, 2022 19:58
@Stephan202 Stephan202 marked this pull request as ready for review August 19, 2022 20:01
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.

Good catch, the subtle change I missed was the return type...

I saw in Platform that the Refaster template for (e.g.) RxJava2Adapter.flowableToFlux(flowable), wasn't changed, so I added them in EPS.
After running the self-check I saw that the IdentityConversion check is supposed to rewrite those. However, this solution is more accurate and better.

@Stephan202
Copy link
Member Author

I saw in Platform that the Refaster template for (e.g.) RxJava2Adapter.flowableToFlux(flowable), wasn't changed, so I added them in EPS.

That's exactly how I started out on this journey as well 😅. But then I noticed that the downstream patch script didn't rewrite those expressions either, so at that the point the question became "why does IdentityConversion trigger in one context, but not the other?". Now we know :)

Copy link

@ibabiankou ibabiankou left a comment

Choose a reason for hiding this comment

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

LGTM, though RxJava/Reactor is not my strongest skill 😅

@rickie rickie merged commit f810530 into master Aug 22, 2022
@rickie rickie deleted the sschroevers/fix-RxJava2Adapter-templates branch August 22, 2022 09:05
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