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

Remove wrong namespace-change of package javax.transaction.xa #554

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

Xaeras
Copy link
Contributor

@Xaeras Xaeras commented Sep 13, 2024

This change removes a wrongly made namespace-change of a package in jakarta-ee-9.yml.

Previously, the package javax.transaction.xa (from Java SE 8) was wrongly changed to jakarta.transaction.xa, which does not exist because it did not change.
This was probably done because the namespace change did affect all sub-packages for other packages of javax.* namespace. With javax.transaction however, there is only this single package in Jakarta EE which has change to jakarta.transaction. The namespace of the package javax.transaction.xa (from Java SE 8) did not change though, which is why my solution would be to just turn the recursive attribute to 'false'.

What's changed?

Changed the value of the recursive attribute in the recipe org.openrewrite.java.migrate.jakarta.JavaxTransactionMigrationToJakartaTransaction from true to false and added a testclass.

What's your motivation?

Months ago I was mentioning this bug in slack here and I forgot to make the MR until now.

Anything in particular you'd like reviewers to focus on?

There is only this single package that has been renamed from javax.transaction to jakarta.transaction as shown here, so i don't think we need the recursive change, especially if it causes unwanted side-effects.

Anyone you would like to review specifically?

I was talking with @timtebeek on slack, so he would probably be the best here.

Have you considered any alternatives or workarounds?

As @timtebeek suggested, I could have excluded the javax.transaction.xa namespace, but I think my solution is more simple. I am open to discussion though :)

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

Removes a wrong package change previously made in jakarta-ee-9.yml. The package javax.transaction.xa was wrongly changed to jakarta.transaction.xa, which does not exist.
@timtebeek timtebeek self-requested a review September 13, 2024 13:34
@timtebeek timtebeek added the bug Something isn't working label Sep 13, 2024
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Tim te Beek <timtebeek@gmail.com>
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 a lot for the detailed test and fix! Should help prevent regressions in the future.

@timtebeek timtebeek merged commit 01888dd into openrewrite:main Sep 13, 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.

2 participants