Skip to content

Fix GH-15796: Add the exit_status() function#19445

Closed
alexandre-daubois wants to merge 1 commit into
php:masterfrom
alexandre-daubois:exit-status-func
Closed

Fix GH-15796: Add the exit_status() function#19445
alexandre-daubois wants to merge 1 commit into
php:masterfrom
alexandre-daubois:exit-status-func

Conversation

@alexandre-daubois
Copy link
Copy Markdown
Member

Fix #15796

Especially useful in shutdown handler, allows to get the current exit status code if set by an exit statement.

Comment thread Zend/tests/exit_status/exit_status_error.phpt Outdated
Comment thread Zend/tests/exit_status/exit_status_basic.phpt Outdated
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

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

Implementation LGTM, but given this is a feature, I've requested reviews from the RMs.

Comment thread Zend/zend_builtin_functions.stub.php
Copy link
Copy Markdown
Member

@DanielEScherzer DanielEScherzer left a comment

Choose a reason for hiding this comment

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

Given the lack of an RFC for this addition, or even a mailing list discussion, the RMs are not comfortable with adding it to PHP 8.5 given the feature freeze.

Marking this as "Request changes" to make it clear that this shouldn't be merged until after PHP 8.5 is branched at the earliest

@alexandre-daubois
Copy link
Copy Markdown
Member Author

Friendly ping on this one, now that PHP 8.5 has been branched

@bukka
Copy link
Copy Markdown
Member

bukka commented Oct 1, 2025

I think it would be good to mention it on internal and describe the use case a bit more. If there are no objections, I think it can be merged.

@DanielEScherzer
Copy link
Copy Markdown
Member

I don't see an option to remove my "requested changes", maybe because of the merge conflict?
Either way, the objection was about including in 8.5 and is no longer applicable

@ndossche
Copy link
Copy Markdown
Member

I don't see an option to remove my "requested changes", maybe because of the merge conflict? Either way, the objection was about including in 8.5 and is no longer applicable

I suppose so, normally the option to dismiss a review is at the bottom. You can rerequest your own review though which I think will also clear the status.

@alexandre-daubois
Copy link
Copy Markdown
Member Author

Just getting back to this one. I'll mention it on the mailing list as soon as I can, as requested by Jakub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

evade exit_status overwrite by exit()

6 participants