-
Notifications
You must be signed in to change notification settings - Fork 116
JDK17 and fixing the dependencies with vulnerabilities #2921
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
Conversation
|
@georgweiss I added some changes to ignore javadoc warnings only from the save and restore service. |
|
@shroffk, I'll try to find time to fix them a.s.a.p. |
I don't think this is that urgent.. no stress :) Plus, we might have to redo some of the doc when we implement the restore on the service anyway... this could be done as part of that project/effort |
georgweiss
left a comment
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.
Should we not update to JavaFX 21 while we're at it?
|
yes, :) I am just trying to make changes step by step to solve issues as they come along... but yes javaFX21 should be part of this change |
georgweiss
left a comment
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.
Also: thumbnail and linear meter modules specify 11 for maven compiler plugin. I think the properties in those pom files should simply be removed.
|
right now I am confused why the test for save-restore-module are not using JDK17 |
JFX21 removes GTK2. JFX20 still allows GTK2 or 3. In the discussion for #2638 that was seen as a possible reason to move only up to JFX20 for a while. |
|
It seemed like the main issues with GTK3 were not reproduceable. At least the rendering issues mentioned by ESS were resolved... I have not yet tested the crashing issues (associated with efxclipse and gtk3) I experienced. At the very least we can move to jfx 20... but it at some point we will have to bite the bullet and make the move |
|
I do not intend to merge this branch immediately... I am trying to have the jdk17 + javafx20/21 branch ready for people to test. |
|
OK, let's try JFX21 then. Since it's really just one number a a top level file, a site that experiences issues can locally build w/ JFX20 to check if that helps. |
|
There is still the open question about jumping to JDK21 also...not sure what our plan should be |
Not sure how you conclude this... |
|
I am seeing this error both in the CI test and on my local tests. The maven -version and the JAVA_HOME are all set correctly but the unit tests still fail with the following. |
|
@shroffk, please remove jacoco plugin in pom. |
|
@shroffk, I'm currently fixing this. Please be patient, will update this PR. |
|
I am working on this too... removing the jacoco plugin from the service too |
|
OK, but I would also fix the Javadoc stuff. |
Brilliant, you should have permission to push to this branch. |
|
@shroffk, I am unable to push to branch error: src refspec jdk-17 does not match any |
|
Can you check that you have the right upstream setup... this seems like a git issue |
|
git push --set-upstream origin jdk-17 This fails... |
|
Where are we with this? Right now the maven build of master is broken: ends in That's with
|
|
Yesterday at the monthly meeting all participants gave the greenlight for the move to JDK17 javafx21 will be part of a separate PR |
|
.. and it builds again, even with maven! What an excellent way to conclude the week!! |
I am getting javadoc warnings which break the build ( when building with docs.. which I think the CI is not doing )
should we just ignore them or fix them... they are all in the save and restore service