-
Notifications
You must be signed in to change notification settings - Fork 35
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
fix: keep projects when restarting the workspace from local devfile #382
Conversation
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request images published ✨ Editor: quay.io/che-incubator-pull-requests/che-code:pr-382-amd64 |
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request images published ✨ Editor: quay.io/che-incubator-pull-requests/che-code:pr-382-amd64 |
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request images published ✨ Editor: quay.io/che-incubator-pull-requests/che-code:pr-382-amd64 |
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request images published ✨ Editor: quay.io/che-incubator-pull-requests/che-code:pr-382-amd64 |
// if a new Devfile does not contain projects, copy them from flattened Devfile | ||
try { | ||
let projects: V1alpha2DevWorkspaceSpecTemplateProjects[] | undefined = devfileContext.devWorkspace.spec!.template!.projects; | ||
if (!projects || projects.length === 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.
Something to consider:
When using the reproducer repo, if you add a new project to your devfile, say:
projects:
- name: web-nodejs-sample
git:
remotes:
origin: "https://github.com/che-samples/web-nodejs-sample.git"
And then restart from the local devfile, you'll have a single-root workspace that only contains the newly added web-nodejs-sample
project. The existing project that we used to restart the workspace from local devfile is no longer shown in VSCode, since its project (that used to be present in the devworkspace) wasn't added to the newly generated devworkspace's projects.
So I suggest:
- Always adding the devworkspace's existing projects
(flattenedDevfile.projects)
to the newly generated devworkspace's projects (devfileContext.devWorkspace.spec!.template!.projects
) instead of only adding it if the new devfile does not contain projects. - Add the devworkspace's existing project(s) to be first in list of projects, since this is the project we had used to restart the workspace from local devfile (it should remain first after we restart)
- Don't add projects with the same name (e.g. if the same project name is used in the devworkspace and local devfile) because this will render the devworkspace invalid.
For reference, the approach I'm suggesting is implemented in DevWorkspace Operator here.
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.
I would give the user all the freedom.
If he added a project to his devfile, he would get only one project in the tree.
Adding a project with the same name should be possible and handled when cloning the project.
In this PR I handled only a case when the user created a workspace from minimal devfile and he wants only to change the devfile a bit and apply the changes.
I would not fix all the cases in this PR. Let's move step by step.
You can check some discussion here eclipse-che/che#22722
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.
Sorry for slow response but yes this makes sense. If changes are needed, we can discuss them in eclipse-che/che#23036 👍
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.
I tested the current PR and can confirm that it fixes the problem that was reported in the eclipse-che/che#23014.
I created a separate issue for the suggestion from @AObuchow (thanks!) - eclipse-che/che#23036.
I reproduced the problem that @AObuchow has described in his comment. I also can reproduce the problem without these changes, so it's no regression.
So, I propose to provide improvement for the Restart From Local Devfile
action after discussion in the eclipse-che/che#23036.
But it's up to @AObuchow @vitaliy-guliy and other reviewers.
Build 3.16 :: code_3.x/1434: Console, Changes, Git Data |
Build 3.16 :: sync-to-downstream_3.x/7175: Console, Changes, Git Data |
Build 3.16 :: get-sources-rhpkg-container-build_3.x/7154: code : 3.x :: Failed in : BREW:BUILD/STATUS:UNKNOWN |
What does this PR do?
What issues does this PR fix?
eclipse-che/che#23014
How to test this PR?
PROJECTS_ROOT
andPROJECT_SOURCE
environment variablesDoes this PR contain changes that override default upstream Code-OSS behavior?
git rebase
were added to the .rebase folder