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

javax.servlet -> jakarta.servlet:6.0 #485

Closed

Conversation

eduarddrenth
Copy link
Contributor

updated dependencies to jakarta
updated groovy accordingly
renamed javax to jakarta

@eduarddrenth
Copy link
Contributor Author

tested this PR on one of my applications, runs fine on payara 6.2022.1 / ee 10

@eduarddrenth
Copy link
Contributor Author

fixes #484

@eduarddrenth
Copy link
Contributor Author

build pipeline needs to support class version 55, run jdk 11

@oliemansm
Copy link
Member

@eduarddrenth That would mean that from this version on Java 8 would no longer be supported by this library. MIght be a good moment as any to do it.

@eduarddrenth
Copy link
Contributor Author

A metter of changing gradle.properties?

SOURCE_COMPATIBILITY=1.8
TARGET_COMPATIBILITY=1.8

to

SOURCE_COMPATIBILITY=11
TARGET_COMPATIBILITY=11

@eduarddrenth
Copy link
Contributor Author

can you perhaps approve, so I can verify the tests succeed with my last commits?

@eduarddrenth
Copy link
Contributor Author

I don't know how to fix the test run issues, servlet runs fine on payara 6. Perhaps lower servlet spec for better compatibility?

@eduarddrenth
Copy link
Contributor Author

solution might be:

  • use springframework 6.0.2 in unit testing
  • add gradle config to run tests with java 17 (required by springframework 6)

Or redesign the test env (large impact I suspect) or skip tests (undesirable)

@oliemansm
Copy link
Member

@eduarddrenth Since this has quite significant impact I've started a poll for this to get some insights. If the poll does not provide a clear direction or doesn't have that many votes I'll make a decision. See #487

@eduarddrenth
Copy link
Contributor Author

@eduarddrenth Since this has quite significant impact I've started a poll for this to get some insights. If the poll does not provide a clear direction or doesn't have that many votes I'll make a decision. See #487

good idea! voted for 17 myself

@eduarddrenth
Copy link
Contributor Author

eduarddrenth commented Dec 2, 2022

Running tests using jdk 17 with

SOURCE_COMPATIBILITY=17
TARGET_COMPATIBILITY=17

and

    testImplementation 'org.springframework:spring-test:6.0.2'
    testRuntimeOnly 'org.springframework:spring-web:6.0.2'

succeeds

@oliemansm
Copy link
Member

@eduarddrenth Can you update your PR with the changes you've confirmed work locally? Can you sync in my latest push to master as well? That should update the github actions to work with jdk 17 only.

@eduarddrenth
Copy link
Contributor Author

fixed some tests

@oliemansm
Copy link
Member

@eduarddrenth I made some changes but couldn't push them to your branch, so created and merged in a separate pr. Closing this one now.

@oliemansm oliemansm closed this Dec 5, 2022
@eduarddrenth
Copy link
Contributor Author

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants