Multi-root by default for Dev Workspaces #1043
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1043 +/- ##
=======================================
Coverage 21.46% 21.46%
=======================================
Files 315 315
Lines 11495 11495
Branches 1710 1710
=======================================
Hits 2467 2467
Misses 8882 8882
Partials 146 146
Continue to review full report at Codecov.
|
| @@ -56,6 +56,11 @@ export class CheWorkspaceServer extends DefaultWorkspaceServer { | |||
| } | |||
|
|
|||
| function isMultiRoot(workspace: Workspace): boolean { | |||
| const devfile = workspace.devfile; | |||
| return !!devfile && !!devfile.attributes && !!devfile.attributes.multiRoot && devfile.attributes.multiRoot === 'on'; | |||
| const devWorkspaceNamespace = process.env.DEVWORKSPACE_NAMESPACE; | |||
There was a problem hiding this comment.
we created abstraction on how workspaces are retrieved (no process.env in the code except in remote-api-services)
so if you need to know, probably you can add a method in the api
another hint is to check if you've runtime in workspace object (undefined in the case of DevWorkspace)
There was a problem hiding this comment.
else, we could always provide the attribute multiRoot in case of DevFile v2 implementation (so we don't change the consumer code)
There was a problem hiding this comment.
Thank you, Florent, for the ideas how to handle this problem.
AFAIK we are going to turn on multi-root by default very soon.
We will consider that - the multi-root is ON for all cases except the case when there is the attribute multiRoot with off value.
So, the check will be like:
function isMultiRoot(workspace: Workspace): boolean {
return workspace.devfile?.attributes?.multiRoot !== 'off';
}
Taking into account the info above I think we should apply a solution from your first comment, so:
- adding a method to the API
or - check if you've runtime in workspace object (undefined in the case of DevWorkspace)
If I didn't missed something - I guess the second one (runtime in workspace object) - is the simplest way.
@benoitf @azatsarynnyy
WDYT?
There was a problem hiding this comment.
yes go to the simplest (2nd case as it's temporary)
|
✅ E2E Happy path tests succeed 🎉
See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>
RomanNikitenko
force-pushed
the
defaultMultiRootForDevWorkspace
branch
from
6314f24 to
390c5aa
Compare
|
✅ E2E Happy path tests succeed 🎉
See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
RomanNikitenko
requested review from
azatsarynnyy,
ericwill,
sunix,
svor,
tsmaeder,
vinokurig,
vitaliy-guliy and
vzhukovs
as code owners
Signed-off-by: Roman Nikitenko rnikiten@redhat.com
What does this PR do?
Enable multi-root by default for Dev Workspaces
Screenshot/screencast of this PR
What issues does this PR fix or reference?
eclipse-che/che#19191
How to test this PR?
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or referenceandHow to test this PRcompletedReviewers
Reviewers, please comment how you tested the PR when approving it.
Happy Path Channel
HAPPY_PATH_CHANNEL=stable