Migrate sanity.yml workflow to setup-java action #144
Conversation
The AdoptOpenJDK/install-jdk has not been updated in a while but setup-java is so it probably makes sense to migrate to it. Also see adoptium#122 for more details.
|
Thanks @ultramancoder ! I have requested a review from @sophia-guo - please count this as one of your Outreachy contributions and we will try to get it reviewed/merged shortly. |
.github/workflows/sanity.yml
Outdated
| @@ -11,16 +11,14 @@ jobs: | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest, windows-latest] | |||
| version: [8, 11, 17] | |||
| impl: [hotspot] | |||
| distribution: [temurin] | |||
There was a problem hiding this comment.
distribution is required by setup-java action. The parameter name can keep as before.
|
Hi! @sophia-guo Thanks for the review. I updated the PR. Did I understand your comment correctly or are more changes needed? |
| @@ -11,16 +11,14 @@ jobs: | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest, windows-latest] | |||
| version: [8, 11, 17] | |||
| impl: [hotspot] | |||
| impl: [temurin] | |||
There was a problem hiding this comment.
The impl value should be hotspot.
There was a problem hiding this comment.
But according to the list here https://github.com/actions/setup-java#supported-distributions, hotspot is not a valid value. Am I missing something here?
There was a problem hiding this comment.
Do you mean that I should leave the impl set to hotspot and add another distribution parameter with value set to temurin and pass that to the action's distribution argument?
There was a problem hiding this comment.
Sorry, you are correct. It should be updated to temurin. I was thinking the impl of TKG.
There was a problem hiding this comment.
Could you tag the PR as ready for review? Thanks
|
Ok |
The AdoptOpenJDK/install-jdk has not been updated in a while but setup-java is so it probably makes sense to migrate to it. Also see #122 for more details.