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

Optional-before-required removal in PHP 8.2 should be documented #44

Open
iansltx opened this issue Jul 6, 2023 · 2 comments
Open

Optional-before-required removal in PHP 8.2 should be documented #44

iansltx opened this issue Jul 6, 2023 · 2 comments

Comments

@iansltx
Copy link

iansltx commented Jul 6, 2023

This isn't really an issue of Invoker, but should_invoke_callable_with_optional_parameter_before_required_parameter fails on PHP 8.2 due to PHP tightening up optional-before-required param behavior. If that's desired functionality in Invoker even >= PHP 8.2 OTOH, code changes will be required. But skipping that test on 8.2, documenting the change, and updating the test grid to include 8.1 and 8.2, seems like a reasonable enough response from here.

@mnapoli
Copy link
Member

mnapoli commented Jul 9, 2023

Thanks, good points!

But skipping that test on 8.2, documenting the change, and updating the test grid to include 8.1 and 8.2, seems like a reasonable enough response from here.

Sounds good, the package shouldn't aim to rewrite native PHP behavior.

@williamdes
Copy link
Contributor

This may be what is causing one test to fail on recent php versions

There was 1 error:

1) Invoker\Test\InvokerTest::should_invoke_callable_with_optional_parameter_before_required_parameter
Invoker\Exception\NotEnoughParametersException: Unable to invoke the callable because no value was given for parameter 1 ($baz)

/home/runner/work/Invoker/Invoker/src/Invoker.php:67
/home/runner/work/Invoker/Invoker/tests/InvokerTest.php:[22](https://github.com/williamdes/Invoker/actions/runs/6120452457/job/16612348312#step:6:23)3

Ref: https://github.com/PHP-DI/Invoker/actions/runs/6221070349/job/16882338119

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants