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

Bump min java version to 11 #1977

Closed
wants to merge 1 commit into from
Closed

Bump min java version to 11 #1977

wants to merge 1 commit into from

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Jan 26, 2022

To support upgrading to lucene 9.x the minimum java version is bumped to java 11.

@nknize
Bump min java version to 11
c083d81 
To support upgrading to lucene 9.x the minimum java version is bumped to java
11.

Signed-off-by: Nicholas Walter Knize <[email protected]>
@nknize nknize added v2.0.0 Version 2.0.0 non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues >upgrade Label used when upgrading library dependencies (e.g., Lucene) labels Jan 26, 2022
@nknize nknize requested a review from a team as a code owner January 26, 2022 19:10
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@reta
Copy link
Collaborator

reta commented Jan 26, 2022 •
edited
Loading

I expected that :D, @nknize, the other thoughts, do you think we could follow Elasticsearch (and many other projects) lead and go full speed on JDK-17? I have crunched JDK vendors for support timelines, JDK-11 is set for EOL in 2023 / 2024 (I am sure it will be around longer but ...), wdyt?

reta
reta requested changes Jan 26, 2022
View reviewed changes
Copy link
Collaborator

@reta reta left a comment

Choose a reason for hiding this comment

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

Please update https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success c083d81
Log 2062

Reports 2062

@nknize
Copy link
Collaborator Author

nknize commented Jan 26, 2022

go full speed on JDK-17

I'm fine w/ that. I think it'll be more involved than bumping the minimum compatibility variables? Since lucene 9 requires a minimum of 11 we can merge this PR and tackle the bump to min 17 in a separate PR?

@reta
Copy link
Collaborator

reta commented Jan 26, 2022 •
edited
Loading

go full speed on JDK-17

I'm fine w/ that. I think it'll be more involved than bumping the minimum compatibility variables? Since lucene 9 requires a minimum of 11 we can merge this PR and tackle the bump to min 17 in a separate PR?

👍 No objections, do you want me to open an issue for discussion?
Thank you!

@andrross
Copy link
Member

go full speed on JDK-17

Maybe continue this discussion in #1689?

@dblock
Copy link
Member

dblock commented Feb 2, 2022

What do we want to do with this PR?

@reta
Copy link
Collaborator

reta commented Feb 2, 2022

@dblock afaik we need to merge it to unblock Lucene 9 migration.

andrross
andrross approved these changes Feb 2, 2022
View reviewed changes
Copy link
Member

@andrross andrross left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This seems right to me. We can merge this into main to unblock the Lucene 9 work while continuing the discussion of whether to go all the way to 17.

We probably want to backport this (11 being minimum supported version) to the 1.x as well given that Java 8 is going end of life.

@reta
Copy link
Collaborator

reta commented Feb 2, 2022

We probably want to backport this (11 being minimum supported version) to the 1.x as well given that Java 8 is going end of life.

This one I wish but not sure, by all means it will be a breaking change for some users, from operations perspective (there are still compelling cases for CMS GC apparently), but out commitment to 1.x - no breaking changes.

@dblock
Copy link
Member

dblock commented Feb 2, 2022

I'd like this PR to include changes to the developer guide and other READMEs purging mentions of Java 8.

@reta
Copy link
Collaborator

reta commented Feb 2, 2022

I'd like this PR to include changes to the developer guide and other READMEs purging mentions of Java 8.

@dblock already asked in here #1977 (review)

@dblock
Copy link
Member

dblock commented Feb 2, 2022

I'd like this PR to include changes to the developer guide and other READMEs purging mentions of Java 8.

@dblock already asked in here #1977 (review)

I have no original ideas, just repeating what other smart people say hoping no one notices :)

@reta
Copy link
Collaborator

reta commented Feb 2, 2022

I'd like this PR to include changes to the developer guide and other READMEs purging mentions of Java 8.

@dblock already asked in here #1977 (review)

I have no original ideas, just repeating what other smart people say hoping no one notices :)

Oops sorry 👍 , emphasizing the importance of keeping docs in sync 🙇

@dblock dblock mentioned this pull request Feb 2, 2022
Merged
1 task
@dblock
Copy link
Member

dblock commented Feb 3, 2022

Superseded with #2025

@dblock dblock closed this Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reta reta reta requested changes

@andrross andrross andrross approved these changes

Assignees
No one assigned
Labels
non-issue bugs / unexpected behaviors that end up non issues; audit trail simple changes that aren't issues >upgrade Label used when upgrading library dependencies (e.g., Lucene) v2.0.0 Version 2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nknize @opensearch-ci-bot @reta @andrross @dblock