-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Replace setup-scala with setup-java #6318
Conversation
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.
IMHO we should keep JVM version 8 as the base version and use it everywhere. Other JVM versions, like 11 or 17 should be then tested only in the testJvms
.
The motivation is that this will make sure that all things work on the widest range of JVM versions -- this is one of Scala's biggest strengths, it is a powerful language that you can run even on an "old" JVM 8.
@@ -99,15 +119,17 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
java: ['11', '17'] | |||
java: ['8', '11'] |
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.
The purpose of the task testJvms
is to test on other JVM versions besides the base one, so it should stay ['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.
It should not, as the base one is now 17.
If I'm not mistaken, that's the whole purpose behind the |
@adamgfraser Could you please sync the |
@mijicd Will do! |
But how about the other actions, like |
|
fair enough 👍 |
Summary