-
Notifications
You must be signed in to change notification settings - Fork 37
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
[2.x] PHP 8.4 support #90
Conversation
Thanks for submitting a PR! Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface. Pull requests that are abandoned in draft may be closed due to inactivity. |
I'd honestly like to wait until it does support php 8.4 so we don't have to use platform-req=php |
Yeah that's why I left it in Draft for now. Just wanted to see if something else's needed to be fixed. |
I'm curious what would cause this error on PHP 8.4 tests https://github.com/laravel/framework/actions/runs/11024490854/job/30617746097 |
@crynobone maybe this is a little helpful (I don't know much about serialising closures yet) Following Code snippet causes the error in the test This is the value of PHP 8.3:
PHP 8.4:
See the small difference Not sure if this helpful, but maybe a starting point for further debugging. |
@crynobone I think this PR is related: How to fix this: serializable-closure/src/Serializers/Native.php Lines 140 to 142 in 43244bf
Instead use the following: $code = $reflector->getCode();
$code = str_replace('\{closure:', '', $code); In the end, I think this is a bug with PHP and should probably be reported. |
I submitted an issue to php-src and they changed the return value of Here's a sample code of how the reflection methods related to closure namespace work in PHP8.4. Now |
to trigger new build
This reverts commit 528e422.
@nunomaduro do you have an idea, why for example the enum/class Pest correctly detects the defined classes / enums in the other PHP versions, but somehow not in PHP 8.4. |
@Jubeki Can you rebase this to |
@crynobone rebase was somehow annoying, I merged develop into this branch. Alls Tests passing 😄 |
Add PHP 8.4 to test matrix.