jenkins-infra / github-reusable-workflows

Repository for reusable workflows
MIT License
1 stars 5 forks source link

Bump Java to 17 #31

Closed lemeurherve closed 4 months ago

lemeurherve commented 4 months ago

This PR bumps java from 11 to 17.

Ref:

FTR, initial PR body: This PR adds an optional input to specify the Java major version to use to run jenkins-maven-cd-action. Its default value is set to `11` for now, should be updated to `17` when Jenkins controllers will require minimum Java 17 version. Example of call to use JDK17: ```diff # Note: additional setup is required, see https://www.jenkins.io/redirect/continuous-delivery-of-plugins name: cd on: workflow_dispatch: check_run: types: - completed permissions: checks: read contents: write jobs: maven-cd: uses: jenkins-infra/github-reusable-workflows/.github/workflows/maven-cd.yml@v1 + with: + java-version: 17 secrets: MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }} MAVEN_TOKEN: ${{ secrets.MAVEN_TOKEN }} ```

Related jenkinsci-dev mailing list thread: https://groups.google.com/g/jenkinsci-dev/c/11P2B9s67tw/m/buquLnLeAwAJ

timja commented 4 months ago

You happy to merge and release @lemeurherve?

create a release for the latest tag using github ui and clicking the github button to add changelog. then retag the v1 tag to match the tag created using github releases

jglick commented 4 months ago

Why not run with 17 unconditionally? Simpler.

lemeurherve commented 4 months ago

Why not run with 17 unconditionally? Simpler.

Might be a stupid question but is it safe with regard to existing plugins targeting only jdk11?

I prefer asking as I don't want to make all plugin cd actions fail.

jglick commented 4 months ago

Yes you should be able to build any plugin using JDK 17. The POM will effectively pass -release 11 to tools. In fact most Jenkinsfiles do not use JDK 11 at all any more (only 17 & 21), and for purposes of the CD release workflow it is even less important to run on the older JDK because the only reason to include JDK 11 in Jenkinsfile would be to run tests on 11 (in case there is some subtle behavioral difference) yet the CD workflow skips all tests.

lemeurherve commented 4 months ago

Thanks for the detailed response!

Changed the pull request to a java version bump.

dduportal commented 4 months ago

I believe this is quite a change of scope for the PR which should be shared on the mailing list + the associated issue.

As devil is in the details, would be worth it to plan and announce this change so at list it would be visible (a PR discussion is never visible except by the author and reviewers).

lemeurherve commented 4 months ago

(EDIT: posted by mistake)

lemeurherve commented 4 months ago

I believe this is quite a change of scope for the PR which should be shared on the mailing list + the associated issue.

As devil is in the details, would be worth it to plan and announce this change so at list it would be visible (a PR discussion is never visible except by the author and reviewers).

I've posted a comment in #30 and asked for opinions in https://groups.google.com/g/jenkinsci-dev/c/11P2B9s67tw/m/buquLnLeAwAJ

timja commented 4 months ago

Released and oh cool I didn't realise the v1 would get automatically moved, that's nice: https://github.com/jenkins-infra/github-reusable-workflows/blob/main/.github/workflows/self-update-major-tag.yml