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.

Sign up for GitHub

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

Jump to bottom

[github-actions] Update to v1.2 and also update actions/checkout to v4 #61

Merged
merged 2 commits into from
Jan 8, 2025
Merged

[github-actions] Update to v1.2 and also update actions/checkout to v4 #61

merged 2 commits into from
Jan 8, 2025

Conversation

SerhatG
Copy link
Member

@SerhatG SerhatG commented Jan 7, 2025

No description provided.

@SerhatG SerhatG requested a review from Hilbrand January 7, 2025 12:52
@SerhatG SerhatG force-pushed the github_actions_update branch from 9f72512 to 63613a7 Compare January 7, 2025 12:56
Hilbrand
Hilbrand approved these changes Jan 8, 2025
View reviewed changes
Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

LGTM

@Hilbrand
Copy link
Member

Hilbrand commented Jan 8, 2025

Actually the JDK 11 is not needed. The pom already specifies source and target java 11, and a JDK 17 can generate Java 11 code. Since JDK only used to build here it should not be a problem to built with Java 17 (I also do that on my development machine).

@SerhatG
Copy link
Member Author

SerhatG commented Jan 8, 2025

Actually the JDK 11 is not needed. The pom already specifies source and target java 11, and a JDK 17 can generate Java 11 code. Since JDK only used to build here it should not be a problem to built with Java 17 (I also do that on my development machine).

Ah, I do know Nashorn was removed in JDK 17, so it's not fully backwards compatible.
GWT used to need this, but not sure if this library does, so wanted to play it safe.
I'll try locally if it works on JDK 17 with 11 as output and change the PR accordingly.

@SerhatG
Copy link
Member Author

SerhatG commented Jan 8, 2025

Confirmed working. Adjusted accordingly

@SerhatG SerhatG merged commit 1061061 into aerius:main Jan 8, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hilbrand Hilbrand Hilbrand approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SerhatG @Hilbrand