-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Use Eclipse Temurin, not AdoptOpenJDK in publish artifact action #6406
Conversation
The 'adopt' distribution has moved to Eclipse Temurin and won't be updated at the AdoptOpenJDK location. https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt describes the transition. https://github.com/actions/setup-java#usage provides more general purpose info. The current Adopt version is reporting: Run actions/setup-java@v2 Resolved Java 8.0.292+1 from tool-cache Setting Java 8.0.292+1 as the default Java configuration: Distribution: adopt Version: 8.0.292+1 Path: /opt/hostedtoolcache/Java_Adopt_jdk/8.0.292-1/x64 The current Temurin version reports: Run actions/[email protected] Resolved Java 8.0.322+6 from tool-cache Setting Java 8.0.322+6 as the default Java configuration: Distribution: temurin Version: 8.0.322+6 Path: /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/8.0.322-6/x64 The upgrade from JDK 8u292 to 8u322 is a positive step, since it is using a JDK version that is 12 months newer.
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.
This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks!
The 'adopt' distribution has moved to Eclipse Temurin and won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
@@ -13,9 +13,9 @@ jobs: | |||
steps: | |||
- uses: actions/checkout@v3 | |||
- name: Set up JDK 8 | |||
uses: actions/setup-java@v2 | |||
uses: actions/setup-java@v3.0.0 |
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.
jenkinsci/file-parameters-plugin#90 (comment) I think also applicable here given
jenkins/.github/workflows/publish-release-artifact.yml
Lines 3 to 5 in abd028b
on: | |
release: | |
types: [published] |
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.
What is the purpose of reviewing a PR that has already been merged?
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.
Just informational, in case the author (or anyone else) thinks it would be appropriate to file a follow-up.
(In this case I did not see this PR until after it is was merged, due to a link from a plugin PR.)
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.
Have you considered doing a quick glance at the PRs with the ready-for-merge
label every day or two to see if you have feedback for them? If the purpose of code review is to make the code of tangibly higher quality, then leaving feedback before a pull request is merged is far more likely to achieve this outcome. To be clear, there are many valid reasons for reviewing a PR after it has been merged, such as to notify authors of serious regressions, but in cases like this where there is no regression and where the suggestion is fairly trivial, it can come across as an example of "bring me a rock" style leadership.
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 am not familiar with that metaphor.
I do not normally attempt to follow all core PRs (or PRs in major plugins for that matter) since I simply lack the time for that sort of volume and there are already a number of diligent reviewers. This one I just stumbled across after the fact, via a link from a PR in a plugin I help maintain.
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yaml Co-authored-by: Tim Jacomb <[email protected]>
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yml Co-authored-by: Tim Jacomb <[email protected]>
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yml Co-authored-by: Tim Jacomb <[email protected]>
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yaml Co-authored-by: Tim Jacomb <[email protected]>
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yaml Co-authored-by: Tim Jacomb <[email protected]>
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yaml Co-authored-by: Tim Jacomb <[email protected]>
* Use Eclipse Temurin, not AdoptOpenJDK in action The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage * Update .github/workflows/cd.yaml Co-authored-by: Tim Jacomb <[email protected]>
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
The 'adopt' distribution has moved to Eclipse Temurin. It won't be updated at the AdoptOpenJDK location. See jenkinsci/jenkins#6406 https://github.com/actions/setup-java/blob/main/docs/advanced-usage.md#adopt https://github.com/actions/setup-java#usage
See jenkinsci/jenkins#6406 for more details The 'adopt' distribution has moved to Eclipse Temurin and won't be updated at the AdoptOpenJDK location. Switch to the Temurin distribution so that we use a current version of Java.
See jenkinsci/jenkins#6406 for more details The 'adopt' distribution has moved to Eclipse Temurin and won't be updated at the AdoptOpenJDK location. Switch to the Temurin distribution so that we use a current version of Java.
See jenkinsci/jenkins#6406 for more details The 'adopt' distribution has moved to Eclipse Temurin and won't be updated at the AdoptOpenJDK location. Switch to the Temurin distribution so that we use a current version of Java. - Bump actions/setup-java from 2 to 3 (dependabot proposed in jenkinsci#300) - Use Eclipse Temurin in the action, not AdoptOpenJDK Supersedes jenkinsci#511
Use Eclipse Temurin, not AdoptOpenJDK in publish artifact action
The 'adopt' distribution has moved to Eclipse Temurin and won't be updated at the AdoptOpenJDK location.
The advanced usage describes the transition.
The setup-java usage provides more general purpose info.
The current Adopt version is reporting:
The current Temurin version reports:
The upgrade from JDK 8u292 to 8u322 is a positive step, since it is using a JDK version that is 12 months newer.
Proposed changelog entries
N/A - skip changelog because there are no user visible results from this change
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgrade@Restricted
or have@since TODO
Javadoc, as appropriate.Desired reviewers
Any
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are accurate, human-readable, and in the imperative moodupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).