Add support for Jakarta servlets#419
Conversation
garydgregory
force-pushed
the
feature/jakarta_servlet
branch
from
f70f653 to
deb3bee
Compare
kiril-keranov
left a comment
kiril-keranov
left a comment
There was a problem hiding this comment.
Thanks for addressing this! The solution suits well, though applications experiencing the issue would still need to make an adjustment they would now have a working variant
| <logback.version>1.3.16</logback.version> | ||
| <slf4j.version>2.0.17</slf4j.version> | ||
| <findsecbugs.version>1.14.0</findsecbugs.version> | ||
| <jakarta.servlet-api.version>5.0.0</jakarta.servlet-api.version> |
There was a problem hiding this comment.
can it be updated to at least 6.0.0?
| */ | ||
| @Override | ||
| public void contextDestroyed(final ServletContextEvent sce) { | ||
| final ClassLoader tccl = Thread.currentThread().getContextClassLoader(); |
There was a problem hiding this comment.
I guess the common code of the javax.* and jakarta.* ServletContextCleaners might be extracted to a common location to prevent duplication
There was a problem hiding this comment.
I was going to do that initially but stopped myself. I think the simplest solution for now is to keep the javax and jakarata paths separate. If some subtle difference needs to be accounted for, then it can be dropped in the right class without concern of creating a regression in the other.
Can you test this PR in your use case?
There was a problem hiding this comment.
I was going to do that initially but stopped myself. I think the simplest solution for now is to keep the
javaxandjakaratapaths separate. If some subtle difference needs to be accounted for, then it can be dropped in the right class without concern of creating a regression in the other.Can you test this PR in your use case?
tested it and worked well
org.apache.commons.logging.jakarta.ServletContextCleaner #419.
|
I merged the PR. Please test again with git master or a snapshot build from https://repository.apache.org/content/repositories/snapshots/commons-logging/commons-logging/1.4.0-SNAPSHOT/ |
@garydgregory tested with both master branch build and this snapshit build. Looks fine |
|
@kiril-keranov |
2 participants
This PR is instead of PR #418 which breaks binary compatibility.
Before you push a pull request, review this list:
[ ] I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?mvn; that'smvnon the command line by itself.