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

fix: dispose the wsl2 command when unregistering extension #5246

Merged
merged 1 commit into from Dec 13, 2023
Merged

fix: dispose the wsl2 command when unregistering extension #5246

merged 1 commit into from Dec 13, 2023

Conversation

benoitf
Copy link
Collaborator

@benoitf benoitf commented Dec 13, 2023

What does this PR do?

register the command to be disposed when removing the extension (fix is at the very bottom of the PR)

Screenshot / video of UI

N/A

What issues does this PR fix or reference?

fixes #4593

How to test this PR?

unit test added

or check testcase from issue

@benoitf
fix: dispose the wsl2 command when unregistering extension
2c71a8e 
fixes containers#4593
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf benoitf requested a review from a team as a code owner December 13, 2023 12:57
@benoitf benoitf requested review from dgolovin and feloy and removed request for a team December 13, 2023 12:57
deboer-tim
deboer-tim approved these changes Dec 13, 2023
View reviewed changes
Copy link
Collaborator

@deboer-tim deboer-tim left a comment

Choose a reason for hiding this comment

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

I don't have a Windows machine to test, but code and tests LGTM and follows pattern we use elsewhere.

@benoitf benoitf mentioned this pull request Dec 13, 2023
Merged
4 tasks
lstocchi
lstocchi approved these changes Dec 13, 2023
View reviewed changes
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

Tested on Win 11!! Stopping/starting the podman extension works fine. Thanks Florent!!

@benoitf benoitf merged commit 856cbce into containers:main Dec 13, 2023
8 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.7.0 milestone Dec 13, 2023
@odockal
Copy link
Contributor

odockal commented Dec 13, 2023

Windows 10 looks good! Thx for fix.

@benoitf benoitf deleted the DESKTOP-4593 branch December 13, 2023 15:05
@slemeur slemeur mentioned this pull request Dec 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deboer-tim deboer-tim deboer-tim approved these changes

@lstocchi lstocchi lstocchi approved these changes

@dgolovin dgolovin Awaiting requested review from dgolovin dgolovin is a code owner automatically assigned from containers/podman-desktop-reviewers

@feloy feloy Awaiting requested review from feloy feloy is a code owner automatically assigned from containers/podman-desktop-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.7.0
5 participants
@benoitf @odockal @deboer-tim @lstocchi @podman-desktop-bot