-
Notifications
You must be signed in to change notification settings - Fork 321
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
Java 17 as minimal version, 2024-03 as minimal platform, updated BOM and minimal dependencies #3233
base: main
Are you sure you want to change the base?
Conversation
Also updated the byte code of Methods.class inside the testData.jar: it is now compiled with Java 17
i assume the following is planned for later
|
Do you mean remove old versions?
That's already implemented in this PR
If you mean remove old tests, then yes, it'll be in another PR
Yes for switches. I don't understand m2e |
javaversion enum m2e: |
Sorry, I don't understand |
i guess we also need to check the java 8 removal to find out what else we did back then |
@HannesWell might be able to explain the m2e stuff or we need to search the old pr |
regaerding emf. this reminds me of the last emf updated. am not sure if we should do everything in one go given the time constraints |
I'd postpone that, and we'll also have to regenerate the languages |
@iloveeclipse @mehmet-karaman i wonder if you could test this branch in your env |
@mehmet-karaman : if you can change our Xtext build to use this branch instead of master, I could try to build platform with that on next Monday and run tests. @cdietrich : I'm out of PC till monday. |
Test Results 6 460 files ± 0 6 460 suites ±0 3h 6m 17s ⏱️ - 3m 10s Results for commit 50c2bf8. ± Comparison against base commit 8321a58. This pull request removes 11 and adds 7 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
The changes are So if you use m2e-2.0+ in your minimal target now you can just call |
@cdietrich I've updated the project wizard tests 5fdf15d I removed the duplicate |
thx @LorenzoBettini |
is it ok when i push the cleanup of TargetPlatformProject? |
What do you mean? |
|
Nice catch! Yes, that needs to be updated as well. |
@LorenzoBettini i have a commit i can just push on your branch |
Please go right ahead |
done |
remove no longer needed reflections
My proposal is to bring this in. Update the reference projects. |
I agree! |
@szarnekow are you available this week? |
We've tested and saw no issues in our automated tests. |
i fear we are running out of time. and @szarnekow does not seem to be reachable ... |
In case, I can build the milestone this week |
Next week is m3 week. And thius we should (tm) be feature complete by then |
So we can build a milestone this week? |
I really had hoped for Sebastian to have a look. If we want to bring it in and considering open points and my availability 2nd half of this week it needs to be asap |
Signed-off-by: Christian Dietrich <[email protected]>
To check
maybe
|
org.eclipse.ui.ide;bundle-version="3.13.1", | ||
org.eclipse.ui.console;bundle-version="3.11.100", | ||
org.eclipse.core.filesystem;bundle-version="1.9.300", | ||
org.eclipse.core.filesystem;bundle-version="1.10.300", | ||
org.eclipse.jdt.ui;bundle-version="3.26.0", |
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.
We should try to be more consistent with the require bundle dependencies, e.g. jdt.ui 3.26 is unlikely to match jdt.core 3.37
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.
@szarnekow I only updated the dependencies when there was a matching one in the dev BOM, IIRC.
In general, I'm OK with keeping in sync minimal versions for the same dependencies that we have in the BOM.
For the other ones, I'd like to avoid such a massive maintenance burden...
org.eclipse.xtext.common.types.ui;bundle-version="2.37.0", | ||
org.eclipse.emf.common;bundle-version="2.24.0", | ||
org.eclipse.emf.common;bundle-version="2.30.0", | ||
org.eclipse.xtext.ui;bundle-version="2.37.0", | ||
org.eclipse.xtext.ui.shared;bundle-version="2.37.0", | ||
org.eclipse.ui.editors;bundle-version="3.14.300", |
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.
Do you know whether there exists a utility that would update all require-bundle versions to match a selected target platform? cc @HannesWell @merks @iloveeclipse
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.
Not that I know of, no.
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.
Do you know whether there exists a utility that would update all require-bundle versions to match a selected target platform?
Would be nice small feature in manifest editor. We have something similar in the feature editor, so may be with some advanced copy/paste work this would be doable.
Minor pending things
|
Moreover,
strictfp
tests had to be adapted (I was wrong about failure only in Java 21:strictfp
has been removed from byte code since Java 17); Concerningorg.eclipse.xtext.common.types.access.impl.AbstractTypeProviderTest.testMethods_publicStrictFpMethod_01()
, I removed it, because in this test project we also use project with sources and 1.8 compilation level. I preferred to keep things as they are, and simply removed this test that wouldn't make sense anymore, anyway.