-
Notifications
You must be signed in to change notification settings - Fork 12
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
switch to javase 21 #656 #659
Conversation
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.
LGTM
...rg.eclipse.mylyn.commons.ui.tests/src/org/eclipse/mylyn/commons/ui/ShellDragSupportTest.java
Show resolved
Hide resolved
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.
It's a huge patch! From what I've seen it should be ready to go. If smth is left if can always be finished and later PRs as moving to Java 21 is mandatory.
What did I miss again @akurtakov ? |
I agree. No one will manually review this much content! No, it isn't required to switch everything to Java 21. But the Lucene dependency generally requires it so definitely Mylyn as a whole cannot work with Java 21, though of course many parts could. Unless there is a somewhat compelling need to continue to work with Java 17, it seems easier to just do mass migration even though it's not required in general. |
If their dependencies allow so. Lucene being kind of at the heart of Mylyn makes it not the case for Mylyn specifically IMO. |
Thank you for explanations @merks @akurtakov
I tried very hard until GitHub started to collapse the contents of the file. Then I switched to check that modifications are done in the right files. Well, not enough, but better than review nothing. |
Change the compiler to JavaSE-21 from JavaSE-17