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

Use and require Java 22 #441

Merged
merged 2 commits into from
Sep 3, 2024
Merged

Use and require Java 22 #441

merged 2 commits into from
Sep 3, 2024

Conversation

oneonestar
Copy link
Member

Description

Close #440
Do not merge yet.

Release notes

(x) Release notes are required, with the following suggested text:

* Require Java 22 for build and runtime.

@cla-bot cla-bot bot added the cla-signed label Aug 21, 2024
@mosabua mosabua mentioned this pull request Aug 21, 2024
Merged
@oneonestar oneonestar force-pushed the star/java_22 branch 5 times, most recently from 031190b to f2d67fc Compare August 22, 2024 00:49
mosabua
mosabua requested changes Aug 23, 2024
View reviewed changes
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Good start. I think we also need to update the docker container build and quickstart docs though. Also maybe jvm config?

@oneonestar
Copy link
Member Author

Docker container build refers to .java-version file, which has been updated.
I didn't find settings related to Java 21/22 in the quickstart or docs.

mosabua
mosabua approved these changes Aug 26, 2024
View reviewed changes
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Looks good to me. Lets get other maintainers to confirm as well

ebyhr
ebyhr reviewed Aug 26, 2024
View reviewed changes
pom.xml Outdated Show resolved Hide resolved
Chaho12
Chaho12 approved these changes Aug 27, 2024
View reviewed changes
Copy link
Member

@Chaho12 Chaho12 left a comment

Choose a reason for hiding this comment

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

LGTM

pom.xml Outdated Show resolved Hide resolved
@wendigo
Copy link
Contributor

wendigo commented Aug 30, 2024

Nice!

@mosabua
Copy link
Member

mosabua commented Aug 30, 2024

@willmostly @vishalya ?

willmostly
willmostly approved these changes Sep 3, 2024
View reviewed changes
Copy link
Contributor

@willmostly willmostly left a comment

Choose a reason for hiding this comment

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

LGTM!

@mosabua
Copy link
Member

mosabua commented Sep 3, 2024

Got confirmation from @vishalya and @andythsu on slack as well. Merging.

@mosabua mosabua merged commit d851e0f into trinodb:main Sep 3, 2024
3 checks passed
@github-actions github-actions bot added this to the 11 milestone Sep 3, 2024
@mosabua mosabua mentioned this pull request Sep 3, 2024
Merged
@oneonestar oneonestar deleted the star/java_22 branch September 26, 2024 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebyhr ebyhr ebyhr left review comments

@Chaho12 Chaho12 Chaho12 approved these changes

@willmostly willmostly willmostly approved these changes

@mosabua mosabua mosabua approved these changes

@wendigo wendigo wendigo approved these changes

@vishalya vishalya Awaiting requested review from vishalya

Assignees
No one assigned
Labels
cla-signed
Milestone
11
Development

Successfully merging this pull request may close these issues.

Use and require Java 22
6 participants
@oneonestar @wendigo @mosabua @ebyhr @willmostly @Chaho12