-
Notifications
You must be signed in to change notification settings - Fork 81
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
Just use 17 (no need for zulu etc.) #542
Conversation
@@ -37,7 +37,7 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
java: ['adopt@1.8', 'adopt@1.11', 'zulu@1.17.0-0'] | |||
java: [adopt@1.8, adopt@1.11, 17] |
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 it's better that we leave zulu here so that we know what is being used. We might want to change to adopt later along the line when it's released.
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.
We might want to change to adopt later along the line when it's released
If we have it as 17
, olafurpg/setup-scala
(transitively jabba
) will do that for us automatically.
But if we keep it as zulu...
, a human will have to remember to come later and make the change -- That's not worth it, IMHO. Most likely, what will happen, is that we will forget about this and will be testing against a pre-release version of Java 17 for years to come 😸
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.
Ach, ok cool. Didn't know that, thanks!
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.
LGTM thank you!
I think the Zulu 17-ea build is quite old but it’s good to have this in place for when the final release is out.
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.
LGTM
No description provided.