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

feat: allow to abort build image (#1655) #4538

Merged
merged 2 commits into from Nov 2, 2023
Merged

Conversation

lstocchi
Copy link
Contributor

What does this PR do?

This is built on #4364

It allows user to abort the build of an image

Screenshot/screencast of this PR

abort_build

What issues does this PR fix or reference?

it closes #1655

How to test this PR?

  1. build an image
  2. abort

@deboer-tim
Copy link
Collaborator

Nit: even though the underlying action is an abort, I think we generally use Cancel for buttons.

@lstocchi lstocchi marked this pull request as ready for review October 27, 2023 13:53
@lstocchi lstocchi requested review from benoitf and a team as code owners October 27, 2023 13:54
@lstocchi lstocchi requested review from jeffmaury and feloy and removed request for a team October 27, 2023 13:54
@lstocchi
Copy link
Contributor Author

Nit: even though the underlying action is an abort, I think we generally use Cancel for buttons.

Updated

@benoitf
Copy link
Collaborator

benoitf commented Oct 30, 2023

it looks like the PR check "linter, formatter, unit tests" is not passing

@lstocchi
Copy link
Contributor Author

it looks like the PR check "linter, formatter, unit tests" is not passing

we need to merge #4364 and rebase this one

@benoitf
Copy link
Collaborator

benoitf commented Oct 30, 2023

ok, I just tried this one and it was not working, cancel button was doing nothing :-)

@axel7083
Copy link
Contributor

Nice @lstocchi !!

Signed-off-by: lstocchi <lstocchi@redhat.com>
Signed-off-by: lstocchi <lstocchi@redhat.com>
@benoitf
Copy link
Collaborator

benoitf commented Oct 30, 2023

I clicked on 'rebase the branch' button as dependent PR has been merged

Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

@lstocchi lstocchi merged commit fe5637b into containers:main Nov 2, 2023
9 checks passed
@lstocchi lstocchi deleted the i1655 branch November 2, 2023 08:51
@podman-desktop-bot podman-desktop-bot added this to the 1.6.0 milestone Nov 2, 2023
@jannikbertram
Copy link
Contributor

hey @lstocchi just wanted to let you know that the test Expect Abort button to being visible when image build is in progress adds a dependency to run tests in the right order. As soon as you swap the last two tests, there is a test failure.

It's probably caused by this

  buildImagesInfo.set({
    buildImageKey: Symbol(),
    buildRunning: true,
  });

Maybe it should set the parameters back to the default after running the test?

@lstocchi
Copy link
Contributor Author

lstocchi commented Nov 3, 2023

Thanks @jannikbertram !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to abort a build of an image
7 participants