Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[task] add ability to restart running task #6811

Merged
merged 1 commit into from
Jan 6, 2020
Merged

[task] add ability to restart running task #6811

merged 1 commit into from
Jan 6, 2020

Conversation

vince-fugnitto
Copy link
Member

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

What it does

Fixes #5330

  • adds a new command and menu item to restart running task.
  • updates the task-service methods restartTask and terminateTask to make them public so
    others may call them.

How to test

  1. start the application using theia/packages/task/test-resources as a workspace.
  2. start the long running test task.
  3. use the command or menu item restart running task to restart the task.
  4. the task should terminate and a new terminal should be opened with the restarted task.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves tasks issues related to the task system labels Jan 2, 2020
@vince-fugnitto vince-fugnitto self-assigned this Jan 2, 2020
elaihau
elaihau approved these changes Jan 3, 2020
View reviewed changes
Copy link
Contributor

@elaihau elaihau left a comment

Choose a reason for hiding this comment

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

I tested this change in Gitpod with a multi root workspace.
Looks good to me.

@vince-fugnitto vince-fugnitto force-pushed the vf/GH-5330 branch 2 times, most recently from dac5341 to ce12455 Compare January 3, 2020 14:30
@RomanNikitenko
Copy link
Contributor

@vince-fugnitto
I'm testing your changes and can see that restart task item is different from run task item for a detected task:
restartItem
But I tested without your update a few minutes ago.

@vince-fugnitto
Add ability to restart running task
de7b06e 
Fixes #5330

- adds a new command and menu item to `restart running task`.
- updates the task-service methods `restartTask` and `terminateTask` to make them public so
others may call them.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto
Copy link
Member Author

@vince-fugnitto
I'm testing your changes and can see that restart task item is different from run task item for a detected task

Thank you, it should be fixed in the latest iteration.

RomanNikitenko
RomanNikitenko reviewed Jan 3, 2020
View reviewed changes
Copy link
Contributor

@RomanNikitenko RomanNikitenko left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@vince-fugnitto
I'm testing your changes and can see that restart task item is different from run task item for a detected task

Thank you, it should be fixed in the latest iteration.

Yes, it's fixed now, thank you!

Did you notice tab details for a restarted task?

restarted_details

Is it expected and useful for tasks?

The behavior with tab details is not related to your changes, I just try to understand is it useful or we should fix it in future

@vince-fugnitto
Copy link
Member Author

Yes, it's fixed now, thank you!

Great!

The behavior with tab details is not related to your changes, I just try to understand is it useful or we should fix it in future

I think it can definitely be improved, what would you suggest?

RomanNikitenko
RomanNikitenko approved these changes Jan 4, 2020
View reviewed changes
Copy link
Contributor

@RomanNikitenko RomanNikitenko left a comment

Choose a reason for hiding this comment

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

I tested the changes for detected tsc tasks and for long running test task - it works well for me!

I found some places to improve, but it's not related to the current PR changes, so I created the separate issues:

@vince-fugnitto vince-fugnitto merged commit 0d5f79a into master Jan 6, 2020
@vince-fugnitto vince-fugnitto deleted the vf/GH-5330 branch January 6, 2020 12:20
@azatsarynnyy azatsarynnyy mentioned this pull request Jan 8, 2020
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elaihau elaihau elaihau approved these changes

@RomanNikitenko RomanNikitenko RomanNikitenko approved these changes

Assignees

@vince-fugnitto vince-fugnitto

Labels
enhancement issues that are enhancements to current functionality - nice to haves tasks issues related to the task system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[tasks] add ability to restart running tasks
3 participants
@vince-fugnitto @RomanNikitenko @elaihau