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

Recipe to call Modifier And ConstantBootstraps methods as static #484

Conversation

ranuradh
Copy link
Contributor

What's changed?

This is a simple recipe for Java 17
org.openrewrite.java.migrate.RemovedModifierAndConstantBootstrapsConstructors
image
The recipe calls org.openrewrite.java.ChangeMethodTargetToStatic

  • 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

@timtebeek
Copy link
Contributor

One question that did arise for me was why these are combined into a single recipe. Something to consider, but not necessarily change before a merge

Copy link
Contributor

@cjobinabo cjobinabo left a comment

Choose a reason for hiding this comment

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

Looks good.

@timtebeek In our tool, we currently have a single migration rule to represent the removal of both constructors. Our migration rules have a 1 to 1 relationship with the OpenRewrite recipes, making it ideal to also have a single recipe to address both constructors. An argument could be made for us to break the rule into 2 separate rules, but I currently don't see the value in doing that.

@timtebeek timtebeek merged commit fcddb6d into openrewrite:main May 22, 2024
2 checks passed
@ranuradh ranuradh deleted the recipe_RemovedModifierAndConstantBootstrapsConstructors branch May 22, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants