Skip to content

Adding all org.glassfish.main migrations to jakarta#517

Merged
timtebeek merged 4 commits intoopenrewrite:mainfrom
sgartner03:main
Jul 29, 2024
Merged

Adding all org.glassfish.main migrations to jakarta#517
timtebeek merged 4 commits intoopenrewrite:mainfrom
sgartner03:main

Conversation

@sgartner03
Copy link
Copy Markdown
Contributor

What's changed?

Added migrations for all Java EE APIs under the organization org.glassfish.main. Added a migration for java.ejb:javax.ejb to jakarta.ejb. These changes are implemented in the org.openrewrite.java.migrate.jakarta.JavaxMigrationToJakarta Recipe.

What's your motivation?

https://rewriteoss.slack.com/archives/C01A843MWG5/p1721228381110469

Any additional context

Should I implement tests for each case? As far as I see the other migrations of this recipe don't have tests and I'm not sure what for I would be testing other than typos anyways.

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
Copy Markdown
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.

Great to see, thanks! Couple of quick questions as I went over the artifactIds; if you could answer those I think we can merge soon after.

For Yaml only recipes it's not always necessary to write a test; in some cases that might bring to light dependencies that can't be found, as might have been the case here. No need to add them now I'd say.

Comment thread src/main/resources/META-INF/rewrite/jakarta-ee-9.yml
Comment thread src/main/resources/META-INF/rewrite/jakarta-ee-9.yml
Comment thread src/main/resources/META-INF/rewrite/jakarta-ee-9.yml
Comment thread src/main/resources/META-INF/rewrite/jakarta-ee-9.yml Outdated
@timtebeek timtebeek added enhancement New feature or request recipe Recipe requested labels Jul 26, 2024
Co-authored-by: Tim te Beek <timtebeek@gmail.com>
@sgartner03 sgartner03 requested a review from timtebeek July 29, 2024 08:20
Copy link
Copy Markdown
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.

Thanks for the work done here and educating me on the changes. :)

@timtebeek timtebeek merged commit 8376fac into openrewrite:main Jul 29, 2024
@sgartner03
Copy link
Copy Markdown
Contributor Author

Thanks for your help! However, I did an oopsie by thinking your thumbs-up in the discussion for org.glassfish:jakarta.faces meant that I should re-add the migration with another target. I think we should revert commit 124cb01 to re-delete the jsf migration.

timtebeek added a commit that referenced this pull request Jul 29, 2024
@timtebeek
Copy link
Copy Markdown
Member

Thanks! Dropped now in 61ed4bc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request recipe Recipe requested

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants