-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add sidecar based maven and gradle Java stacks #12898
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
Please consider applying my proposals just to have tags in the same style
I guess the plan is also to replace it for rh-che, right ? |
Nice. I also saw a companion PR to update an existing sample: https://github.com/che-samples/console-java-simple |
I restarted workspace with Java plugin several times, changed default RAM, but still no java assistance. Is it known bug? Do we want to replace stack with such a state? |
@slemeur it may break versions of Che that use a version of JDK older than JDK 8 (2014). Is that an issue?
@garagatyi agree that's annoying. I couldn't make it work neither. That's not related to JDT LS but to any vs code extension running in a sidecar. I have been pointed to eclipse-theia/theia#3970. I was hoping that it could be fixed before this PR got merged because anyway it's critical. @benoitf @tolusha any comments about that? That said if that cannot be fixed by the end of the sprint I am ok to not replace this stack but just create a distinct one (i.e. By the way this make me think that even if we fix the JDT LS loading problem we still have the following limitations (added that to the issue description):
|
@l0rd about eclipse-theia/theia#3970 / plug-ins reload I'm working on it since the last couple of days and made several fixes. Another fix is pending eclipse-theia/theia#4583 |
@garagatyi @benoitf cool thank you for the update on those issues. So it looks like the situation is under control. @benoitf it would be great if you could test this java stack with your patch (see if JDT LS work fine) or if you could point me to a che-theia to use to test. My main concern with this PR is QE team anyway. @rhopp I would like to merge it as it is to turn the page and don't think about Che 6 java stack anymore :-) But if that has a huge impact on your test I can keep (maybe hiding it) the existing default java stack. |
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
Co-Authored-By: l0rd <mario.loriedo@gmail.com>
@l0rd I had issues on 'plug-in reload' with vscode-java extension and fixed it upstream redhat-developer/vscode-java#832 |
No, it's not 👍 Thanks for clarifying! |
ci-test |
Results of automated E2E tests of Eclipse Che Multiuser on OCP: |
Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/1634//Selenium_20tests_20report/) showed regression in selenium tests from Failed selenium tests will be updated by #12913 after this PR is merged. |
Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
For the record I will update this PR to keep the old GWT based java-default stack. I updating it because JDT LS is never loaded when using this stack hence there is no Java support yet (it should be related to #12904). |
@SkorikSergey I have updated the PR to keep the old java-default stack. We cannot replace the old stack until we have proper debugging support with JDT LS running in a sidecar (#12446) and I would like to merge this PR in the meantime. Is it ok for you to merge? |
@l0rd It's Ok for me. There will need to make small changes to selenium tests. I will do it after merging. |
@l0rd I am coming a bit late on this one: |
@sunix there is a thread in che-dev (Custom builds and side cars) where we discuss that. You may want to have a look at it and participate to the conversation? |
Results of automated E2E tests of Eclipse Che Multiuser on OCP: |
What does this PR do?
Some choices made to build the new stacks:
che-theia:next
is used instead oflatest
${current.project.path}
placeholder for commands doesn't work on Theia and I have replaced it with${CHE_PROJECTS_ROOT}/<sample-name>
Current Limitations
This PR is related to che-samples/console-java-simple#7
@rhopp with that PR the good old che 6 java stack is gone. I suspect this will have some side effects on QE...