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

ExitWithError() - final push to strict mode #22690

Merged
merged 2 commits into from
May 14, 2024

Conversation

edsantiago
Copy link
Member

@edsantiago edsantiago commented May 13, 2024

Final followup to #22270. That PR added a temporary convention
allowing a new form of ExitWithError(), one with an exit code
and stderr substring. In order to allow bite-size progress,
the old no-args form was still allowed. This PR removes
support for no-args ExitWithError().

This PR also adds one piece of new functionality: passing ""
(empty string) as the stderr arg means "expect exit code
but fail if there's anything at all in stderr".

And, one small commit because I seem to have missed a few.

None

@openshift-ci openshift-ci bot added do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels May 13, 2024
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Copy link

Cockpit tests failed for commit f194bc459cc04a0aa89640f9ee54450a2e452e9a. @martinpitt, @jelly, @mvollmer please check.

@edsantiago edsantiago force-pushed the exitwitherror-final branch from f194bc4 to 79cb62c Compare May 13, 2024 18:19
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Copy link

Cockpit tests failed for commit 79cb62c841216b086ebed0bcf4d2ef1d838510e8. @martinpitt, @jelly, @mvollmer please check.

Final followup to containers#22270. That PR added a temporary convention
allowing a new form of ExitWithError(), one with an exit code
and stderr substring. In order to allow bite-size progress,
the old no-args form was still allowed. This PR removes
support for no-args ExitWithError().

This PR also adds one piece of new functionality: passing ""
(empty string) as the stderr arg means "expect exit code
but fail if there's anything at all in stderr".

Signed-off-by: Ed Santiago <[email protected]>
@edsantiago edsantiago force-pushed the exitwitherror-final branch from 79cb62c to d4e40fe Compare May 13, 2024 19:59
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Copy link

Cockpit tests failed for commit d4e40fe. @martinpitt, @jelly, @mvollmer please check.

@rhatdan
Copy link
Member

rhatdan commented May 13, 2024

LGTM
@Luap99 PTAL

Copy link
Member

@Luap99 Luap99 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 openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 14, 2024
Copy link
Contributor

openshift-ci bot commented May 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, Luap99

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

@openshift-merge-bot openshift-merge-bot bot merged commit 5044a0b into containers:main May 14, 2024
88 of 91 checks passed
@edsantiago edsantiago deleted the exitwitherror-final branch May 14, 2024 11:25
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Aug 13, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Aug 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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
Development

Successfully merging this pull request may close these issues.

3 participants