Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Migrate to sbt-typelevel #2857
Migrate to sbt-typelevel #2857
Changes from 1 commit
dfa10d1
b46b7ed
190aff5
e419eff
fc1b9ed
9706fcf
226dc55
c402a52
e4b8afc
e1aae6d
1cf00a0
b6f95d6
4bc1595
d36af8e
d3659d0
b0b4a99
2d1dc57
44bf4e1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Hmm, I guess these are baked in a little too hard :) anyway should be harmless.
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.
Bleh. :-) I'd definitely like to see these cleaned up, but it's kind of okay for now.
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.
Why is the Graal version not specified. This feels a little weird in general since it suggests that the GraalVM version would be either unstable or unconfigurable.
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 can revert this. Most people probably want the latest Graal and not a specific version (which is easy to forget to update, as we've forgotten to here :)
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.
Btw @vasilmkd what are your thoughts on this topic, maybe we need to revise the JDK index to support this?
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 were already on latest, I was doing it manually. Graalvm also follows the JDK release cycle, so as soon as the latest drops, the previous one is obsolete and unsupported. I don't want to sound dismissive, but I'm not interested in changing the index to support old versions.
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 see the latest in the index is 22.0.0.2 and CE is on 21.3.0, so we've fallen behind again (at least on this branch).
Ok @djspiewak your call, how do you want to configure this for CE?
It depends on what you are targeting. If you are targeting JDK whatever then shouldn't it be as stable as any of the other JVMs?
OTOH, if you are targeting specific GraalVM APIs then obviously it makes a difference. Actually I have a project like this.
FTR I didn't remove the ability to configure a specific GraalVM (it still shells out to DeLaGuardo/setup-graalvm like you set it up) although it is now a little more "discouraged" because I expect 99% of projects don't actually want this.
Finally, if you truly want stability, you should really pin the JDK index to a particular hash anyway :)
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.
sbt-typelevel already does some of these exclusions for us. Rather than tease apart what's been upstreamed we'll just keep using our own.
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 shoudl try to inherit more in the future, but this is a fine start.