Skip to content

Conversation

BhavanaPidapa
Copy link
Contributor

What's changed?

image

What's your motivation?

I used org.openrewrite.java.ChangeType for the recipe RemovedPolicy

Anyone you would like to review specifically?

@cjobinabo @timtebeek

Have you considered any alternatives or workarounds?

Since it was not possible to test the recipe. I created sample files, created a local plugin using
./gradlew publishToMavenLocal then ran it in the Java8 Sample App using mvn rewrite:dryRun

Attaching the rewrite.patch file
rewrite.patch

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

@timtebeek timtebeek requested review from cjobinabo and timtebeek and removed request for cjobinabo June 13, 2024 07:55
@timtebeek timtebeek added the recipe Recipe requested label Jun 13, 2024
@@ -1,5 +1,5 @@
#
# Copyright 2022 the original author or authors.
# Copyright 2024 the original author or authors.
Copy link
Member

Choose a reason for hiding this comment

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

For any future PR: no need to update these copyright headers; we typically keep the year on when they were first introduced.

Copy link
Member

@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.

Neat addition @BhavanaPidapa, thanks for adding the recipe and verifying it locally. Indeed it can be difficult to unit test classes removed in newer Java versions, but this should be ok.

One small suggestion to update the phrasing of the displayed name and description, but otherwise good to merge from my end. I'll leave it up to @cjobinabo to do a final check and merge.

Co-authored-by: Tim te Beek <timtebeek@gmail.com>
@cjobinabo
Copy link
Contributor

cjobinabo commented Jun 14, 2024

I've gone ahead and commited the suggestion. Thanks for looking at this @timtebeek

@timtebeek
Copy link
Member

Great! Then feel free to merge whenever suits you best. :)

@timtebeek timtebeek changed the title Recipe RemovedPolicy Replace javax.security.auth.Policy with java.security.Policy Jun 14, 2024
@cjobinabo cjobinabo merged commit c223f32 into openrewrite:main Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

recipe Recipe requested

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants