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

Revert "Add support for 'podman pod' on FreeBSD" #16114

Closed
wants to merge 1 commit into from
Closed

Revert "Add support for 'podman pod' on FreeBSD" #16114

wants to merge 1 commit into from

Conversation

baude
Copy link
Member

@baude baude commented Oct 11, 2022 •
edited
Loading

Signed-off-by: Brent Baude [email protected]

Does this PR introduce a user-facing change?


NONE

PR #16083 breaks pod creation

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 11, 2022
@baude
Revert "Add support for 'podman pod' on FreeBSD"
cbcde14 
PR containers#16083 breaks pod creation

[NO NEW TESTS NEEDED]

Signed-off-by: Brent Baude <[email protected]>
vrothberg
vrothberg approved these changes Oct 11, 2022
View reviewed changes
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 11, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude, vrothberg

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ashley-cui
ashley-cui reviewed Oct 11, 2022
View reviewed changes
Copy link
Member

@ashley-cui ashley-cui left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Oct 11, 2022
@mheon
Copy link
Member

mheon commented Oct 11, 2022

How does it break pod creation?

@baude
Copy link
Member Author

baude commented Oct 11, 2022

How does it break pod creation?

run it ... but the point is it breaks things. Figure out why later

@mheon
Copy link
Member

mheon commented Oct 11, 2022

I presume @dfr is going to try this again after this, and I'd like to give him at least some information about why the original PR failed so he'll have an easier time of it next time?

@baude
Copy link
Member Author

baude commented Oct 11, 2022

this is no longer the case. closing

@baude baude closed this Oct 11, 2022
@dfr
Copy link
Contributor

dfr commented Oct 12, 2022 •
edited
Loading

I am not sure exactly what happened here - what was the problem?

These refactoring PRs do come with risk and that risk is greatly reduced by the excellent test coverage in podman. In cases like this, backing out the PR is the right move but please also capture details of the problem in an issue, ideally including reproduction steps. This can help to improve test coverage and avoid similar problems in future.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ashley-cui ashley-cui ashley-cui left review comments

@vrothberg vrothberg vrothberg approved these changes

Assignees

@ashley-cui ashley-cui

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@baude @mheon @dfr @vrothberg @ashley-cui