- Notifications
You must be signed in to change notification settings - Fork2.9k
Fixes #27121. quadlet: add Pod field to quadlet list output#27552
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Honny1 left a comment
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.
Code LGTM. Please add a test for listing quadlets to this file:https://github.com/containers/podman/blob/main/test/system/253-podman-quadlet.bats
TomSweeneyRedHat commentedNov 18, 2025
Code LGTM |
0108ce7 to0709f88Comparecodynguyen-dev commentedNov 19, 2025
@Honny1 just pushed the test! let me know how it looks |
Honny1 left a comment
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 checked the new test. Overall, it looks good, but it fails. I added an explanation of where the problem is.
test/system/253-podman-quadlet.bats Outdated
| assert "$output" =~ $'standalone.container[[:space:]]*$' "standalone container should have empty pod field" | ||
| assert "$output" =~ $'test-pod.pod[[:space:]]*$' "pod itself should have empty pod field" |
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.
The test fails because$ matches the end of the entire$output string. If you want to match the end of the line, you need to use\n.
mheon commentedNov 20, 2025
/approve |
| | .App| Name of application if Quadlet is part of an app| | ||
| | .Name| Name of the Quadlet file| | ||
| | .Path| Quadlet file path on disk| | ||
| | .Pod| Pod name (if the quadlet is part of a pod| |
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.
Recommend changing toPod name (if the quadlet is part of a pod - only applicable to .container files)
mheon commentedNov 20, 2025
I have a documentation nit. Code LGTM. |
Fixes:containers#27121Signed-off-by: codynguyen-dev <codynnn@outlook.com>
0709f88 to668817eComparecodynguyen-dev commentedNov 21, 2025
Honny1 left a comment
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,
PTAL at the CI failure@timcoding1988
[APPROVALNOTIFIER] This PR isAPPROVED This pull-request has been approved by:codynguyen-dev,Honny1,mheon The full list of commands accepted by this bot can be foundhere. The pull request process is describedhere Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. SeeCONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?