Skip to content

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jan 31, 2025

The last usage within php-src was within PDO but this is no longer the case since 3ff7758 Moreover, external usage is very limited and the two usages I could find have been removed. [1][2]

[1] zephir-lang/zephir@82a8d05
[2] krakjoe/uopz@b1015ae

The last usage within php-src was within PDO but this is no longer the case since 3ff7758
Moreover, external usage is very limited and the two usages I could find have been removed. [1][2]

[1] zephir-lang/zephir@82a8d05
[2] krakjoe/uopz@b1015ae
@Girgias Girgias marked this pull request as ready for review January 31, 2025 16:15
@Girgias Girgias requested a review from dstogov as a code owner January 31, 2025 16:15
Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

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

I don't see anything bad in keeping this API.
May be some third party extensions use it.

@DanielEScherzer
Copy link
Member

Maybe deprecate in 8.5 and remove in 8.6?

Copy link
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.

Consider this an approval in concept.

Comment on lines +22 to +27
. The zend_fcall_info_args_ex() API has been removed.
This is because it does not follow the usual CUFA semantics and it
automatically wraps by-value arguments into references for by-ref
parameters.
It is recommended to set the FCI named_params instead,
however zend_fcall_info_args() can also be used.
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see a little more actionable description of what to do when previously passing a non-NULL func for some reason.

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.

4 participants