Skip to content
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

fix GHA problems #558

Merged
merged 3 commits into from
Jun 23, 2023
Merged

fix GHA problems #558

merged 3 commits into from
Jun 23, 2023

Conversation

mzuehlke
Copy link
Collaborator

closes: #557

* make sure ids don't contain DOTs
* openj9 is adopt-openj9
https://github.com/actions/setup-java#supported-distributions
@durban
Copy link

durban commented Jun 23, 2023

Thanks, this makes sense.

According to the note below the table you linked, "It is highly recommended to migrate workflows from adopt and adopt-openj9, to temurin and semeru respectively...". Would it make sense to use "semeru" instead of "adopt-openj9"?

@mzuehlke
Copy link
Collaborator Author

According to the note below the table you linked, "It is highly recommended to migrate workflows from adopt and adopt-openj9, to temurin and semeru respectively...". Would it make sense to use "semeru" instead of "adopt-openj9"?

Looking at the table I would suggest to:

  • deprecate Distribution.OpenJ9
  • add Distribution.Semuru
  • add Distribution. Microsoft

WDYT ?

@armanbilge
Copy link
Member

Thanks, I agree. I also wonder if we should deprecate the legacy way of specifying Graal versions.

@mzuehlke
Copy link
Collaborator Author

Thanks, I agree. I also wonder if we should deprecate the legacy way of specifying Graal versions.

With legacy, you mean this one, right:

final case class GraalVM(version: String) extends Distribution(version)

@durban
Copy link

durban commented Jun 23, 2023

  • I'd be fine with Distribution.OpenJ9 simply using semeru (as far as I understand, they're practically the same). But deprecating and creating Distribution.Semeru is also fine.
  • Adding microsoft: sure, why not.
  • Deprecating legacy graal versioning: as far as it's possible to use it, I don't mind.

To avoid again having non-working JavaSpecs, it might make sense to add all these to the CI (unless of course having too much is a concern).

@armanbilge
Copy link
Member

  • I'd be fine with Distribution.OpenJ9 simply using semeru (as far as I understand, they're practically the same)

Feels a bit disingenuous. After all, upstream could have chosen to do that, but they didn't.

  • Deprecating legacy graal versioning: as far as it's possible to use it, I don't mind.

Well, maybe I take this back. Might be too early to deprecate this, seems the old versions may still be supported for a while.

To avoid again having non-working JavaSpecs, it might make sense to add all these to the CI (unless of course having too much is a concern).

Yeah, annoying to bloat up the CI, but I don't disagree. Sorry for the regressions.

Copy link
Member

@armanbilge armanbilge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added them to CI and everything that is here looks good. We can follow-up with additional distributions/deprecations etc. Thanks a bunch!

@armanbilge armanbilge merged commit 80d4fce into main Jun 23, 2023
@armanbilge armanbilge deleted the fix_java-setup branch June 23, 2023 20:09
@mzuehlke
Copy link
Collaborator Author

Added them to CI and everything that is here looks good. We can follow-up with additional distributions/deprecations etc. Thanks a bunch!

Thanks for finishing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certain JVM versions doesn't work in github-actions
3 participants