-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: support java-version-file #95
base: main
Are you sure you want to change the base?
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
@@ -12,6 +13,8 @@ export const INPUT_CACHE = 'cache' | |||
export const INPUT_CHECK_FOR_UPDATES = 'check-for-updates' | |||
export const INPUT_NI_MUSL = 'native-image-musl' | |||
|
|||
export const DISTRIBUTIONS_ONLY_MAJOR_VERSION = ['corretto'] |
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.
corretto
does not make sense here, I'm ok with an empty array for compatibility with the upstream code
export const DISTRIBUTIONS_ONLY_MAJOR_VERSION = ['corretto'] | |
export const DISTRIBUTIONS_ONLY_MAJOR_VERSION = [] |
Action is able to recognize all variants of the version description according to [jenv](https://github.com/jenv/jenv). | ||
Valid entry options: | ||
``` | ||
major versions: 8, 11, 16, 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.
8, 11, 16 are rather old. Could you please update this and the other lists to 17+?
major versions: 8, 11, 16, 17 | ||
more specific versions: 1.8.0.2, 17.0, 11.0, 11.0.4, 8.0.232, 8.0.282+8 | ||
early access (EA) versions: 15-ea, 15.0.0-ea, 15.0.0-ea.2, 15.0.0+2-ea | ||
versions with specified distribution: openjdk64-11.0.2 |
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.
openjdk64
does not make sense here. Maybe use graalvm-21.0.3
or so?
Thanks for the PR, @v1v. It'd be great if you could sign the OCA but in the meantime, I left a couple of comments. We'll do a full review when your OCA signature is confirmed. |
Thanks @fniephaus for the review, I'll look into it
I did it, it seems my company had an OCA and I added this project, but no clue what else I need to do |
Thank you!
Maybe @alina-yur can help? :) |
hi @v1v. Can you please resubmit the request using your company email? |
Thank you for signing the OCA. |
Similarly done in actions/setup-java#426