Skip to content

Conversation

rgcv
Copy link

@rgcv rgcv commented Aug 23, 2025

Following this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a GitHub issue filed
    for the change (usually before you start working on it). Trivial changes like typos do not
    require a GitHub issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [#XXX] - Fixes bug in SessionManager,
    where you replace #XXX with the appropriate GitHub issue. Best practice
    is to use the GitHub issue title in the pull request title and in the first line of the commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • add fixes #XXX if merging the PR should close a related issue.
  • Run mvn verify to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If you have a group of commits related to the same change, please squash your commits into one and force push your branch using git rebase -i.
  • Committers: Make sure a milestone is set on the PR

Trivial changes like typos do not require a GitHub issue (javadoc, comments...).
In this case, just format the pull request title like [DOC] - Add javadoc in SessionManager.

If this is your first contribution, you have to read the Contribution Guidelines

If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement
if you are unsure please ask on the developers list.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.


Based on the overall complex changes introduced by #2017 (and discussion within it) and #2018, I decided to open a more concise PR that runs the OpenRewrite Jakarta EE 10 migration recipe.

As a result, the base commit is the changeset introduced by applying the recipe. Surrounding commits handle a few things manually, such as

  • Removing the jakarta classifier from shiro artifacts + shade-plugin configuration
  • Tackle minor checkstyle issues resulting from the migration
  • Remove deprecated HttpSessionContext polyfill and related code (gone in EE10)
  • Removed some stray duplicate jax-rs dependency declarations
  • Temporarily(!) disable some support modules, integration tests, and samples

Regarding the last bullet point, perhaps even per supported module/feature-set, a few more changes are required. Plus, some decisions might want to be made before proceeding with migration, namely:

  • Are we still supporting such integrations? (E.g., ehcache, which would need a major bump, as seen in [#1585] Migrate to Jakarta EE 10 (3.x) #2018; guice, which is already on v7; spring(-boot) which require a major bump and java 17, etc.)
  • Should this PR be worked on iteratively? (there are a few more OpenRewrite recipes worth exploring. Notoriously, spring-boot migration recipes also include quite a few gems such as java 17 uplift, and the Jakarta EE10 one, which was already applied)

@lprimak
Copy link
Contributor

lprimak commented Aug 25, 2025

Thank you once again @rgcv

It would be great if you could give exact commands that you ran to get to this PR. This way I can independently reproduce this PR and check the (much smaller) diffs manually. Can you please do that? This would greatly simplify and speed up the review process.

@rgcv
Copy link
Author

rgcv commented Aug 25, 2025

It would be great if you could give exact commands that you ran to get to this PR.

Sure thing! There was mostly just one, responsible for the first commit alone, which was

./mvnw -U org.openrewrite.maven:rewrite-maven-plugin:run \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-migrate-java:RELEASE \
  -Drewrite.activeRecipes=org.openrewrite.java.migrate.jakarta.JakartaEE10 \
  -Drewrite.exportDatatables=true

as per https://docs.openrewrite.org/recipes/java/migrate/jakarta/jakartaee10#usage.

The commits following that were man-made, after running a series of verifys and compiles to detect what portions of code seemed broken by the application of the migration.

@lprimak
Copy link
Contributor

lprimak commented Aug 25, 2025

Awesome, thank you! I think will finally have a way forward with the ability to properly review now.

@lprimak lprimak added java Pull requests that update Java code CLA 3.x valid Disable automation for valid issues labels Aug 26, 2025
@bmarwell bmarwell requested review from lprimak and bmarwell August 27, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x CLA java Pull requests that update Java code valid Disable automation for valid issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants