-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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: List container with volume filter #16934
Conversation
bf1d449
to
4bab7bc
Compare
/approve |
Should a test be added? |
@Luap99 Should a test be added? |
@SamirPS Please add a test. |
@rhatdan Please tell me to what file I should add the test. |
I would look at test/system/160-volumes.bats. |
LGTM |
Okay, thanks for the information waiting for the merge then. |
/lgtm |
Oops i thinks i have wrongly done my squash |
Yes, there's still a merge commit. |
Yeah but when i squash this commit I have modified file :45 |
Try |
Modify the condition in line 149 in order to list container by mounting point. Closes containers#16019 Signed-off-by: SamirPS <[email protected]>
@vrothberg Thanks for the command, it's good 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.
/lgtm
/hold
Yes, there merge commit is gone now. Thank you!
Thanks you for the command! waiting for the test now |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, mheon, SamirPS The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@vrothberg @edsantiago @rhatdan Can I remove the SLASHhold or not ? I because the int remote fedora-27 failed because of the network test. |
DO NOT REMOVE IT. |
I'm sorry I wasn't clear. This is the rule: you can remove the hold only when tests go green. You cannot ever ever remove it otherwise. The failure is a flake, it is rerunning. Once it goes green, I or someone else will remove the hold. Since it seems I can't explain properly, and there's a possibility of confusion, please do not touch anything for now. Thank you. |
Okay sorry, i understand now |
No problem. It's complicated and unintuitive. If you look at CI right now or in the next few minutes, you'll see that the flaking test has passed, but there are still a bunch more tests running. That's because CI is a cascade: certain tests will only run if their precursors pass. Here's a visualization. So now we still have another hour or so to wait to see if CI goes green. Anyhow, the critical thing to remember is: CI must be green in order to merge. |
All green right now |
Indeed! /hold cancel Thank you @SamirPS ! |
Modify the condition in line 149 in order to list container by mounting point.
Closes #16019
Signed-off-by: SamirPS [email protected]
Does this PR introduce a user-facing change?
This PR only permit to fix the issue #16019