-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Support for java 17 in generated projects - Codestarts #20849
Support for java 17 in generated projects - Codestarts #20849
Conversation
...main/resources/codestarts/quarkus/buildtool/gradle-kotlin-dsl/base/build-layout.include.qute
Outdated
Show resolved
Hide resolved
...esources/codestarts/quarkus/tooling/dockerfiles/base/src/main/docker/Dockerfile.tpl.qute.jvm
Show resolved
Hide resolved
It looks pretty good, don't forget to squash it to 1 commit |
Thanks for the quick review, but it's still on WIP. I will do some more testing as soon as i finish i notice for a second review |
@netodevel the JDK 17 is now available. Instead of installing the package, we are going to use a different base image (as suggested by @cescoffier on zulip):
We should also use a openjdk runtime base image for previous versions. As this is a priority, if you are not available, I will continue the dev for this later today on this PR (but you'll keep the credit for your work of course), let me know.. |
I prepared a commit with the changes: ia3andy@fdf6124 But it seems that jdk 17 is on a different repository IMO having to login or use a token to pull the image is a blocker. |
...rc/main/resources/codestarts/quarkus/tooling/dockerfiles/base/Dockerfile-layout.include.qute
Outdated
Show resolved
Hide resolved
I pushed my commit, this PR needs to be squashed and rebased from Java 17 Runtime is in the process of being pushed to the unauthenticated repository: This PR will have to wait for this registry.access.redhat.com/ubi8/openjdk-17-runtime to be available. |
...rc/main/resources/codestarts/quarkus/tooling/dockerfiles/base/Dockerfile-layout.include.qute
Show resolved
Hide resolved
...s/devtools-common/src/main/java/io/quarkus/devtools/project/codegen/CreateProjectHelper.java
Outdated
Show resolved
Hide resolved
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 8c32b9a
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: independent-projects/tools/devtools-testing
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/gradle/gradle-model and 5 more 📦 independent-projects/tools/devtools-testing✖
✖
✖
✖
✖
✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: independent-projects/tools/devtools-testing
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/gradle/gradle-model and 5 more 📦 independent-projects/tools/devtools-testing✖
✖
✖
✖
✖
✖
⚙️ JVM Tests - JDK 17 #- Failing: independent-projects/tools/devtools-testing
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/gradle/gradle-model and 5 more 📦 independent-projects/tools/devtools-testing✖
✖
✖
✖
✖
✖
|
e17db85
to
1865ac8
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 1865ac8
Failures⚙️ Initial JDK 11 Build #- Failing: independent-projects/tools/devtools-common
! Skipped: core/deployment core/test-extension/deployment core/test-extension/runtime and 640 more 📦 independent-projects/tools/devtools-common✖ |
@netodevel you need to run |
f47f277
to
c338e50
Compare
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building c338e50
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: independent-projects/tools/devtools-testing
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/gradle/gradle-model and 5 more 📦 independent-projects/tools/devtools-testing✖
✖
✖
✖
✖
✖
⚙️ JVM Tests - JDK 11 Windows #- Failing: independent-projects/tools/devtools-testing
! Skipped: devtools/cli devtools/gradle/gradle-application-plugin devtools/gradle/gradle-model and 5 more 📦 independent-projects/tools/devtools-testing✖
✖
✖
✖
✖
✖
|
It seems you need to fix the tests snapshot files (by running them with |
...s/codestarts/quarkus/tooling/dockerfiles/base/src/main/docker/Dockerfile.tpl.qute.legacy-jar
Show resolved
Hide resolved
02ec0e9
to
86030b3
Compare
...ing/src/test/java/io/quarkus/devtools/codestarts/quarkus/QuarkusCodestartGenerationTest.java
Show resolved
Hide resolved
...s/devtools-common/src/main/java/io/quarkus/devtools/project/codegen/CreateProjectHelper.java
Outdated
Show resolved
Hide resolved
...main/resources/codestarts/quarkus/buildtool/gradle-kotlin-dsl/base/build-layout.include.qute
Outdated
Show resolved
Hide resolved
...starts/src/main/resources/codestarts/quarkus/buildtool/gradle/base/build-layout.include.qute
Outdated
Show resolved
Hide resolved
...s/devtools-common/src/main/java/io/quarkus/devtools/project/codegen/CreateProjectHelper.java
Outdated
Show resolved
Hide resolved
...s/devtools-common/src/main/java/io/quarkus/devtools/project/codegen/CreateProjectHelper.java
Outdated
Show resolved
Hide resolved
Adding support to java versions lts Updated template of builds for support java 17 Fixs imports Remove sysout Change codestart dockerfiles to use openjdk-xx-runtime Remove no longer needed RUN_JAVA_VERSION Fixs code formatting Fixs of code review Remove the conversion to integer from the java_version Remove java 8 and set java 11 to default Fixs tests of codestarts generation
345b2a5
to
c24179a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Great job!
...rc/main/resources/codestarts/quarkus/tooling/dockerfiles/base/Dockerfile-layout.include.qute
Show resolved
Hide resolved
...rc/main/resources/codestarts/quarkus/tooling/dockerfiles/base/Dockerfile-layout.include.qute
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are good to go.
Thanks a lot @netodevel!!! This was not an easy one to take to completion 🍾🥳
If we have problems we can always iterate from this.. |
@ia3andy, Thanks a lot for all the help you gave me :) |
IIUC, the base image has been changed, correct? If so, this is a breaking change and we should label it as such. |
Fixes #20752