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.

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

Add a rerun last task command #62645

Merged
merged 8 commits into from
Nov 20, 2018
Merged

Add a rerun last task command #62645

merged 8 commits into from
Nov 20, 2018

Conversation

alexr00
Copy link
Member

@alexr00 alexr00 commented Nov 6, 2018

Fixes #25310

@alexr00 alexr00 self-assigned this Nov 6, 2018
@alexr00 alexr00 requested a review from dbaeumer November 6, 2018 09:16
@alexr00 alexr00 added this to the November 2018 milestone Nov 6, 2018
@@ -85,6 +83,7 @@ export const enum TaskExecuteKind {
export interface ITaskExecuteResult {
kind: TaskExecuteKind;
promise: TPromise<ITaskSummary>;
task: Task;
Copy link
Member

Choose a reason for hiding this comment

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

Can you quickly explain why we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

In handleExecuteResult in task.contribution.ts we need the Task that was executed. For tasks that were run using rerun, task.contribution.ts has no knowledge of the task. Putting the task in the execute result solves this.

Copy link
Member

@dbaeumer dbaeumer left a comment

Choose a reason for hiding this comment

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

LGTM. Only one questions.

With these changes you will own the terminal task system :-)

Copy link
Member

@dbaeumer dbaeumer left a comment

Choose a reason for hiding this comment

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

Actually there is on thing we should think about: would it make sense to let users control what rerunning means in a per task basis. I could for example imagine that rerunning some tasks should reevaluate the vars and rerunning some other should not. So would be cool if I can control this with a property on the task itself. The question will then be what the default is. I would actually argue to should rerun and reevaluate.

@alexr00
Copy link
Member Author

alexr00 commented Nov 7, 2018

As a task property, I agree that the default should be to rerun and reevaluate. Having the task revaluate seems like a logical step from Run to Rerun, while having it as a setting gives the user more power to use the already evaluated variables. I'll make this change.

@dbaeumer
Copy link
Member

Would it make sense to introduce a RunOption and have a property in there and move the isBackground there as well. I am indifferent as well but top level things on the task might get messy at some time.

@alexr00 alexr00 merged commit 6bb7714 into master Nov 20, 2018
@alexr00 alexr00 deleted the alexr00/rerunTaskSquashed branch November 20, 2018 08:41
@hendrics
Copy link

@alexr00 this is a long awaited feature! Thank you for this!

One quick question though - there are no tests for this feature except adding a custom section, or does it do more?

@alexr00
Copy link
Member Author

alexr00 commented Nov 20, 2018

I didn't write any new tests for this command, that's correct. You do not need to add anything to your tasks.json to try it out with the default rerun behavior. The default behavior is that all task related variables will be re-evaluated. If you want to make your task run faster and you always want the same variables as the previous run, you can set the runOptions in your tasks.json to have "rerunBehavior":"useEvaluated"

@hendrics
Copy link

I am looking forward to trying it when it’s released! I am just curious it was accepted with no automated tests. I am wondering if it is a newer tendency and automated tests no longer considered a best practice?
Nevertheless thank you for implementing it! I am just concerned this or other functionality might regress. And I use vscode everyday :-)!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-run Last Task
3 participants