Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

WIP Make Che use multi root workspaces by default #408

Closed

Conversation

JPinkney
Copy link
Contributor

What does this PR do?

This PR makes Che use multi root workspaces by default.

It is still WIP because there are underlying issues with the vscode-languageclient-node library that causes a race condition between when a project is cloned and when the didUpdateWorkspaceFolders is enabled. See: microsoft/vscode-languageserver-node#451

Related issues for C# and PHP support:
eclipse-che/che#14223
eclipse-che/che#14225

This PR will be considered mergable when the vscode-languageserver-node fix is in and all the stacks are behaving as expected.

Currently, this is status of the stacks:

Stack First project works Second project works Issues/Comments
Java-maven 🚫 🚫 still same classpath issue
Golang 👍 👍 everything is working as expected
PHP 👍 👍 everything is working as expected
PHP Symphony 👍 👍 everything is working as expected
Python (without clicking "Try it now" in the popup) 👍 👍 everything is working as expected
Python (after clicking "Try it now" in the popup) ⚠️ 🚫 Document symbols, hover, format, autocompletion worked on the first project. Validation did not. On second project import I have to re-click "Try it now" and then it says a process is now listening on port 2503
NodeJS React 👍 👍 everything is working as expected
.NET Core 🚫 🚫 No features are working with first project or second project.

Testing involved using the sample and adding in:

  - type: cheEditor
    reference: >-
      https://gist.githubusercontent.com/JPinkney/fd653195c71604e3b3002e46393b0f18/raw/1240df21746c56ac4ff66ecd8ee3a9767b074e68/reference.yaml

as the cheEditor in the devfile. That referenced yaml is the result of building a che-theia image with these changes.

Then I waited for clone to finish, tested out features. Then added in a second project (either by cloning or by creating a new folder in /projects) and tested the features on that. For Golang, Python and NodeJS React I just made new sample project myself inside of the /projects folder and added them to the workspace. For PHP I cloned https://github.com/banago/simple-php-website.

What issues does this PR fix or reference?

Part of the fixes for eclipse-che/che#13427

Release Notes

Docs PR

@che-bot
Copy link
Contributor

che-bot commented Aug 19, 2019

Can one of the admins verify this patch?

@dmytro-ndp
Copy link
Contributor

crw-ci-test

@eclipse-che eclipse-che deleted a comment from che-bot Sep 4, 2019
@che-bot
Copy link
Contributor

che-bot commented Sep 4, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

if (cloneCommand.isInTheiaWorkspace()) {
workspaceFolders.push(theia.Uri.file(cloneCommand.folder));
}
cloneCommand.clone();
Copy link
Contributor

Choose a reason for hiding this comment

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

cloneCommand.execute ?

Copy link
Contributor

@sunix sunix Jan 3, 2020

Choose a reason for hiding this comment

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

I am not in favor of having logic implementations here. It should go to the command implementation.

@@ -68,13 +69,13 @@ export class TheiaCloneCommand {
this.projectsRoot = projectsRoot;
}

execute(): PromiseLike<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't see any good reasons to rename execute to clone.
This is a command. At the moment we tell Che-theia to clone (running clone commands) at start up but we may chain other commands that are not about cloning.

);
await theia.commands.executeCommand('che.workspace.addFolder', workspaceFolders);
Copy link
Contributor

@sunix sunix Jan 3, 2020

Choose a reason for hiding this comment

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

same comments i had for factory-initializer

I am not in favor of having logic implementations here. It should go to the command implementation.

You see here you are duplicating code.
Also, workspace root folders may be handle differently in the future see: eclipse-che/che#15347

@benoitf
Copy link
Contributor

benoitf commented Aug 10, 2020

currently superseed by #778
archiving

@benoitf benoitf closed this Aug 10, 2020
@JPinkney JPinkney deleted the workspace_roots_che_on_master branch August 10, 2020 11:48
@JPinkney JPinkney restored the workspace_roots_che_on_master branch August 10, 2020 11:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants