-
Notifications
You must be signed in to change notification settings - Fork 289
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(Tasks): task lifecycle managed in main #8144
fix(Tasks): task lifecycle managed in main #8144
Conversation
eccc79a
to
73ddc86
Compare
73ddc86
to
a8a9ba1
Compare
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 see some changes that can't be done
- Tasks leaking into API (we have notifications here, not tasks)
- Tasks creation that have purely removed from renderer part (it says "to ease the PR") but it can't be merged using that approach.
I can place it in the
@benoitf my proposal was to make a follow-up PR which would replace those by creating them in the main. I will include them inside this PR; the list are available here #7396 (comment) Here is a video demo with the latest commit Screencast.from.2024-07-23.13-49-46.mp4 |
908791d
to
3863310
Compare
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.
Is it just me or the progressing tasks are not displayed? E.g if i stop a podman machine i don't see anything in the task manager
You were right, some tasks were missing. I updated the list of frontend tasks in #7396 (comment) I think this PR is too big. I will put into draft, and see how to split it. |
85ddcad
to
f6d1031
Compare
f6d1031
to
04dd5dd
Compare
rebased after #8292 |
588be29
to
2d0a344
Compare
Rebased after #8534 |
it looks like we have some invalid code (unit test failing) overall LGTM, lot of work 👍 |
Yeah, some change introduced in other PRs appear after rebase. Fixed now ✔️ |
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.
no changes in the external/extension-api so 👍
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
…ovider status change etc.) Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
Signed-off-by: axel7083 <42176370 [email protected]>
4024bcb
to
d2214eb
Compare
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.
LGTM
🔎 What does this PR do?
Makes all the Tasks being managed in the main.
📑 Notable change
new ipcs
tasks:clear-all
->window.clearTasks(): void
tasks:clear
->window.clearTask(taskId: string): void
tasks:execute
->window.executeTask(taskId: string): void
Interfaces change
Creating
TaskInfo
andNotificationTaskInfo
in theapi
package.Creating
Task
andNotificationTask
in theextension-api
TaskManager change
taskManager.updateTask(task)
as the Task intercept all setter insideTaskImpl
class, it is automatically managed and propagated🔨 Future work
This PR remove all the tasks created on the renderer side WITHOUT replacing them by equivalent on the
main
. This choice is to ease the testing of this PR.The list of tasks to created has been listed here #7396 (comment)
🖼️ Screenshot / video of UI
task-management-backend-3.mp4
🐛 What issues does this PR fix or reference?
Fixes #7396
Prior work for #7709
Requires
How to test this PR?
🪲 Debug commands palette
You can add a series of debug commands to the palette to generate fake tasks.
Press
F1
to open Command Palette