Export configurations of Che tasks to config file #295

RomanNikitenko · 2019-06-19T10:00:03Z

What does this PR do?

The changes the same as in the PR #355
We need to cherry-pick #367 and #373 after merge this PR.

Export configurations of Che tasks to config file
commands section of devfile can contain actions with che commands, for example:

commands:
- name: task-plugin:watch
  actions:
  - type: exec
    component: che-dev
    command: >
              yarn watch
    workdir: /projects/che-theia/plugins/task-plugin

- name: che-theia:build
  actions:
  - type: exec
    component: che-dev
    command: >
              yarn
    workdir: /projects/che-theia

The PR allows to convert them to theia tasks format and copy to config file of current workspace folder. After that they are available for running from UI: Terminal => Run Task menu or using My Workspace panel. So now we consider tasks.json file as one source where user can find all che tasks, edit them or add a new task.

The current implementation allows to merge existed configurations from config file and configs from devfile. So user can add new configurations to config file or edit existed ones - the changes are stored to config file.

What issues does this PR fix or reference?

The PR to 7.0.0 branch: #355
eclipse-che/che#13392
https://github.com/eclipse/che-theia/issues/248

fixes for Che commands the issue: Can not run Configured task eclipse-theia/theia#5064
We provided che commands as detected theia tasks. The issue was caused by changes in theia project (the description contains reference to the code) and was reproduced at moving a task from detected to configured section. The issue is not relevant as now che tasks are citizens of tasks.json file and available for editing without moving from detected to configured section.
fixes for Che commands the issue: Recently used tasks section should respect configured tasks eclipse-theia/theia#5067
The issue was manifested when user run a task after configuring from recently used section. The original configuration was run, not overrided. It’s not relevant anymore - che tasks have only one source now - tasks.json file.
fixes for Che commands the issue: Contributed Task Configuration question eclipse-theia/theia#4928
The issue was relevant for detected tasks, according to the changes of this PR we consider che commands as configured theia tasks, so it’s not relevant for che commands anymore.

How to test

Use the component in your devfile

- 
    alias: theia-editor
    reference: >-
      https://raw.githubusercontent.com/RomanNikitenko/che-plugin-registry/master/v3/plugins/eclipse/che-theia/next/meta.yaml
    type: cheEditor

You can use commands section above as example to provide some tasks in your config file.
Try to run che commands using Terminal => Run Task menu, edit a task configuration in tasks.json file.
Note: My Workspace panel is not working at the moment: eclipse-che/che#13956

Signed-off-by: Roman Nikitenko rnikiten@redhat.com

sunix · 2019-06-19T12:56:23Z

Hello Roman, could you explain what this is doing? what is the context? what is the benefit for the end user? which flow it would improve?
Thank you

RomanNikitenko · 2019-06-19T14:32:47Z

Hello Roman, could you explain what this is doing? what is the context? what is the benefit for the end user? which flow it would improve?
Thank you

updated description for the PR
please, let me know if you have some questions

sunix · 2019-06-19T16:48:17Z

thanks
Can't we just say that if a task already exists, we don't do anything ? rather than adding _1 at each start ?

sunix · 2019-06-19T20:51:42Z

otherwise, i still don't understand the meaning of configured or detected ... it doesn't mean anything for an ordinary developer.

RomanNikitenko · 2019-06-20T06:46:40Z

otherwise, i still don't understand the meaning of configured or detected ... it doesn't mean anything for an ordinary developer.

these are not Che terms, these are VS Code terms

RomanNikitenko · 2019-06-20T07:45:40Z

thanks
Can't we just say that if a task already exists, we don't do anything ? rather than adding _1 at each start ?

We don't add prefix each start, we check each start.
We add prefix if we have conflict only.

don't do anything

Could you clarify what do you mean?
User starts workspace from devfile with Che commands, we convert them to Theia tasks and export to config file.
Suppose, user edited a task in config file, but didn't change label.
Also user has ability to edit the task in devfile without changing label.
At the next start/refresh he has Che commands from devfile and tasks defined in config file - he has conflict.
So, what do your propose for this case?

thanks in advance!

slemeur · 2019-06-20T07:53:59Z

@RomanNikitenko Can you list the scenario you tested this capability against?

sunix · 2019-06-20T10:20:21Z

@RomanNikitenko , could we

update the devfile each time you do a change on the config file (tasks.json?) (sync config file -> devfile)
override the config file anytime we can (start ? ) from the devfile (partial sync devfile -> config file)

that would cover 95% of the usecase. It would be simpler to implement and simpler to use, simpler to maintain. Ideally, we should be able to be notified each time the devfile is modified but we can't right now.

If you could also just not talk about VSCode terms that do not match with a Che context. Does VSCode has remote containers ? does VSCode provides commands/tasks from a devfile ? does VSCode create workspaces on demand ? no ... so you can just copy VSCode in Che context. it doesn't always work. Che is not VSCode, try not just copy VSCode but make a great IDE for our community and our customers.

RomanNikitenko · 2019-06-20T10:51:41Z

If you could also just not talk about VSCode terms that do not match with a Che context. Does VSCode has remote containers ? does VSCode provides commands/tasks from a devfile ? does VSCode create workspaces on demand ? no ... so you can just copy VSCode in Che context. it doesn't always work. Che is not VSCode, try not just copy VSCode but make a great IDE for our community and our customers.

I'm really sorry, I can not understand how it's related to my answer above.
Could you point me quote above which caused your reaction?

sunix · 2019-06-20T15:01:23Z

If you could also just not talk about VSCode terms that do not match with a Che context. Does VSCode has remote containers ? does VSCode provides commands/tasks from a devfile ? does VSCode create workspaces on demand ? no ... so you can just copy VSCode in Che context. it doesn't always work. Che is not VSCode, try not just copy VSCode but make a great IDE for our community and our customers.

I'm really sorry, I can not understand how it's related to my answer above.
Could you point me quote above which caused your reaction?

yes sure

otherwise, i still don't understand the meaning of configured or detected ... it doesn't mean anything for an ordinary developer.

these are not Che terms, these are VS Code terms

RomanNikitenko · 2019-06-20T16:20:58Z

If you could also just not talk about VSCode terms that do not match with a Che context. Does VSCode has remote containers ? does VSCode provides commands/tasks from a devfile ? does VSCode create workspaces on demand ? no ... so you can just copy VSCode in Che context. it doesn't always work. Che is not VSCode, try not just copy VSCode but make a great IDE for our community and our customers.

I'm really sorry, I can not understand how it's related to my answer above.
Could you point me quote above which caused your reaction?

yes sure

otherwise, i still don't understand the meaning of configured or detected ... it doesn't mean anything for an ordinary developer.

these are not Che terms, these are VS Code terms

I'm sorry, Sun, it wasn't deliberate. I just use these terms because user can see a task on UI as 'configured' or 'detected'

Also I tried to explain that the PR changes the way how we provide Che commands as Theia tasks.
The difference is 'detected' vs 'configured', this is the main idea of the PR.
So, do you have ideas how I should name them? I'll do it.
Thank you and please accept my apologies!

sunix · 2019-06-20T16:40:27Z

:) @RomanNikitenko please don't be sorry, i just try to share my point of view.

So what does detected or configured mean for a che command task ? a detected che command, a configured che command. I have no idea ... so what would an ordinary developer think ?

RomanNikitenko · 2019-07-04T07:16:53Z

:) @RomanNikitenko please don't be sorry, i just try to share my point of view.

So what does detected or configured mean for a che command task ? a detected che command, a configured che command. I have no idea ... so what would an ordinary developer think ?

The basic difference is:

'detected' tasks come using plugin API
'configured' tasks come from the config file.

Do you have any idea how we can simplify an ordinary developer's life?

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

RomanNikitenko · 2019-08-16T13:13:16Z

I rebased the PR, please review, the changes are the same as in the PR for 7.0.0

olexii4 · 2019-08-20T10:16:20Z

plugins/task-plugin/src/export/launch-configs-exporter.ts

    private getConfigFileUri(rootDir: string): string {
        return resolve(rootDir.toString(), CONFIG_DIR, LAUNCH_CONFIG_FILE);
    }
+
+    private saveConfigs(launchConfigFileUri: string, content: string, configurations: theia.DebugConfiguration[]) {


Add a return type

fixed, thank you!

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

RomanNikitenko · 2019-08-21T10:36:08Z

@olexii4 @vitaliy-guliy
thank you very much for the review!

so I'm merging the PR

RomanNikitenko requested review from olexii4, sunix and vitaliy-guliy as code owners June 19, 2019 10:00

RomanNikitenko mentioned this pull request Jul 10, 2019

Running any task fails after refresh eclipse-che/che#13799

Closed

RomanNikitenko force-pushed the exportCheCommands branch from 0fe8fa0 to d7f88b6 Compare July 21, 2019 15:15

RomanNikitenko requested review from l0rd, rhopp and vparfonov as code owners July 21, 2019 15:15

RomanNikitenko mentioned this pull request Jul 26, 2019

Edited tasks in Theia does not work eclipse-che/che#13508

Closed

Export configurations of Che tasks to config file

064b624

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

RomanNikitenko mentioned this pull request Aug 16, 2019

Can not invoke Task if source is che eclipse-che/che#14254

Closed

RomanNikitenko force-pushed the exportCheCommands branch from d7f88b6 to 064b624 Compare August 16, 2019 12:18

vitaliy-guliy mentioned this pull request Aug 20, 2019

Run Che command in selected container if container doesn't specified in configuration #274

Closed

olexii4 reviewed Aug 20, 2019

View reviewed changes

olexii4 approved these changes Aug 20, 2019

View reviewed changes

Add return type for methods

32cab27

Signed-off-by: Roman Nikitenko <rnikiten@redhat.com>

vitaliy-guliy approved these changes Aug 21, 2019

View reviewed changes

RomanNikitenko merged commit e9fcf75 into master Aug 21, 2019

RomanNikitenko deleted the exportCheCommands branch August 21, 2019 10:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export configurations of Che tasks to config file #295

Export configurations of Che tasks to config file #295

RomanNikitenko commented Jun 19, 2019 •

edited

Loading

sunix commented Jun 19, 2019

RomanNikitenko commented Jun 19, 2019

sunix commented Jun 19, 2019 •

edited

Loading

sunix commented Jun 19, 2019

RomanNikitenko commented Jun 20, 2019

RomanNikitenko commented Jun 20, 2019

slemeur commented Jun 20, 2019

sunix commented Jun 20, 2019 •

edited

Loading

RomanNikitenko commented Jun 20, 2019 •

edited

Loading

sunix commented Jun 20, 2019

RomanNikitenko commented Jun 20, 2019

sunix commented Jun 20, 2019 •

edited

Loading

RomanNikitenko commented Jul 4, 2019

RomanNikitenko commented Aug 16, 2019

olexii4 Aug 20, 2019

RomanNikitenko Aug 20, 2019

RomanNikitenko commented Aug 21, 2019

Export configurations of Che tasks to config file #295

Export configurations of Che tasks to config file #295

Conversation

RomanNikitenko commented Jun 19, 2019 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

How to test

sunix commented Jun 19, 2019

RomanNikitenko commented Jun 19, 2019

sunix commented Jun 19, 2019 • edited Loading

sunix commented Jun 19, 2019

RomanNikitenko commented Jun 20, 2019

RomanNikitenko commented Jun 20, 2019

slemeur commented Jun 20, 2019

sunix commented Jun 20, 2019 • edited Loading

RomanNikitenko commented Jun 20, 2019 • edited Loading

sunix commented Jun 20, 2019

RomanNikitenko commented Jun 20, 2019

sunix commented Jun 20, 2019 • edited Loading

RomanNikitenko commented Jul 4, 2019

RomanNikitenko commented Aug 16, 2019

olexii4 Aug 20, 2019

Choose a reason for hiding this comment

RomanNikitenko Aug 20, 2019

Choose a reason for hiding this comment

RomanNikitenko commented Aug 21, 2019

RomanNikitenko commented Jun 19, 2019 •

edited

Loading

sunix commented Jun 19, 2019 •

edited

Loading

sunix commented Jun 20, 2019 •

edited

Loading

RomanNikitenko commented Jun 20, 2019 •

edited

Loading

sunix commented Jun 20, 2019 •

edited

Loading