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

Do not convert return types in NoGuavaJava21 by default #601

Merged
merged 14 commits into from
Nov 25, 2024

Conversation

BramliAK
Copy link
Contributor

@BramliAK BramliAK commented Nov 13, 2024

What's changed?

Rollback visit Variable Declarations

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Some suggestions could not be made:

  • src/main/java/org/openrewrite/java/migrate/guava/AbstractNoGuavaImmutableOf.java
    • lines 19-19

@timtebeek
Copy link
Contributor

hi @BramliAK ; Understand that you'd like this fixed; Instead of completely removing this, can we make this opt-in with an @Option instead? That way the nice addition you added in #590 isn't completely lost, and it opens the door to an alternative composite recipe that does potentially unsafe replacements.

timtebeek and others added 3 commits November 14, 2024 10:23
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@timtebeek timtebeek self-requested a review November 25, 2024 21:51
@timtebeek timtebeek added the bug Something isn't working label Nov 25, 2024
@timtebeek timtebeek changed the title [HotFix] NoGuavaJava21 causing invalid code since v2.28.0 Do not convert return types in NoGuavaJava21 by default Nov 25, 2024
Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Thanks @BramliAK ! Good to be back to stable recipe outputs by default, while still being able to do the unsafe migrations if desired.

@timtebeek timtebeek linked an issue Nov 25, 2024 that may be closed by this pull request
@timtebeek timtebeek merged commit 59d56a6 into openrewrite:main Nov 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

NoGuavaJava21 causing invalid code since v2.28.0
2 participants