Deprecate current.project.path variable #585
Conversation
Signed-off-by: Vladyslav Zhukovskyi <vzhukovs@redhat.com>
vzhukovs
requested review from
l0rd,
olexii4,
rhopp,
sunix and
vitaliy-guliy
as code owners
|
❌ E2E Happy path tests failed ❗
See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1) ℹ️ |
| @@ -38,33 +38,7 @@ export class ProjectPathVariableResolver { | |||
| return this.onError('Projects root is not set'); | |||
| } | |||
|
|
|||
| const selections = await theia.commands.executeCommand<Uri[]>(SELECTED_CONTEXT_COMMAND); | |||
There was a problem hiding this comment.
Maybe we should not remove this logic if at the moment it's just deprecated?
For example, user starts a workspace from devfile with two defined projects: theia and che-theia.
By default user has /projects as workspace folder.
According to these changes ${current.project.path} returns /projects, not /projects/theia for example.
Isn't it?
Please correct me if I'm wrong.
There was a problem hiding this comment.
This command relies on the current selection. When workspace opens at first time, the selection is not initialized and we're getting error message ... bad substitution .... It will return /project only when user opened explorer part or some file in editor.
There was a problem hiding this comment.
selections in case of opening workspace at first time will be undefined. So it useless and calling ${workspaceFolder} will return the same result if editor or explorer is opened.
There was a problem hiding this comment.
I understand that there are bugs related to using this variable if user has incorrect selection, that's why we decided to use ${workspaceFolder} instead of ${current.project.path}.
But if user or our samples rely on ${current.project.path}, these places will be broken after your changes!
isn't is?
There was a problem hiding this comment.
Our samples don't use this macro. If user uses this macro in own devfile, it will broken anyway and command won't execute. There will be created another PR into docs that describes, that fact, that this macro is deprecated and has to be replaced with another one. So, yes, this changes proposal changes behavior of the flow for this macro.
There was a problem hiding this comment.
steps to reproduce:
- start a workspace from a devfile, get project structure like:
- run the task:
{
"type": "che",
"label": "test2",
"command": "sleep 3 && echo ${workspaceFolder}",
"target": {
"workingDir": "/projects"
}
},
output: /projects
- select
che-theiaproject and run the task:
{
"type": "che",
"label": "test2",
"command": "sleep 3 && echo ${current.project.path}",
"target": {
"workingDir": "/projects"
}
}
output: /projects/che-theia
| const PROJECTS_ROOT_VARIABLE = 'CHE_PROJECTS_ROOT'; | ||
| const ERROR_MESSAGE_TEMPLATE = 'Can not resolve \'current.project.path\' variable.'; | ||
| /** | ||
| * Contributes the variable for getting path for current project as a relative path to the first directory under the root workspace. | ||
| * | ||
| * @deprecated will be removed soon. Use ${workspaceFolder} macro instead. |
There was a problem hiding this comment.
I think it's not enough just replace ${current.project.path} by ${workspaceFolder}. Instead user should use ${workspaceFolder}/theia for example
There was a problem hiding this comment.
There will be created PR into docs that describes that fact, that this macro is deprecated and has to be replaced
|
Closing this PR due to rework the mechanism of returning the value for current project path. There will be another PR. |
* update to latest mta/rhamt 0.0.46 (#17680) Change-Id: I3227d764850fb18f8d9053f438be6f1cfb8e31ef Signed-off-by: nickboldt <nboldt@redhat.com> * bump sha to ca5adb5786114250fb9d4cebe6a327fd8d4d793c (0.0.46) Change-Id: I9fe451bf80904c6866b1afc64fe0a5124b224ada Signed-off-by: nickboldt <nboldt@redhat.com>
What does this PR do?
For the current state
${current.project.path}is redundant and with this changes proposal marks as deprecated. User may use${workspaceFolder}instead.Signed-off-by: Vladyslav Zhukovskyi vzhukovs@redhat.com
What issues does this PR fix or reference?
eclipse-che/che#13636
Release Notes
Deprecate
${current.project.path}macroDocs PR