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

Adjust pretty name of closures on PHP 8.4 #6351

Merged
merged 2 commits into from
Nov 19, 2024
Merged

Conversation

staabm
Copy link
Contributor

@staabm staabm commented May 25, 2024

Introduction

related to php/php-src#13550

see analog symfony change: symfony/symfony#54614

Relevant issues

Changes

API changes

Behavioural changes

Backwards compatibility

Follow-up

Tests

I tested this PR by doing the following (tick all that apply):

  • Writing PHPUnit tests (commit these in the tests/phpunit folder)
  • Playtesting using a Minecraft client (provide screenshots or a video)
  • Writing a test plugin (provide the code and sample output)
  • Other (provide details)

@ShockedPlot7560
Copy link
Member

PHP 8.4 is not officially supported

@staabm
Copy link
Contributor Author

staabm commented May 25, 2024

I was just crawling thru github for this particular php compat problem.

Its not important for me to make this repo PHP 8.4 compatible

@dktapps
Copy link
Member

dktapps commented May 31, 2024

I think this would be better filled by using ReflectionFunction::isAnonymous in 8.2 and up

@staabm
Copy link
Contributor Author

staabm commented May 31, 2024

I think this would be better filled by using ReflectionFunction::isAnonymous in 8.2 and up

thats right, but it would require raising the min-php version requirement

@jasonw4331 jasonw4331 added Category: Core Related to internal functionality Type: Fix Bug fix, typo fix, or any other fix labels Jun 23, 2024
@dktapps
Copy link
Member

dktapps commented Aug 13, 2024

Not if we gate it with a PHP_VERSION_ID check. This change makes me uneasy. This check was sketchy enough to begin with.

@dktapps
Copy link
Member

dktapps commented Nov 19, 2024

I don't like this, but it's less work than trying to get PHPStan to not complain about any version-conditional logic I tried to implement...

@dktapps dktapps merged commit e710b37 into pmmp:stable Nov 19, 2024
13 checks passed
@staabm staabm deleted the patch-1 branch November 19, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core Related to internal functionality Type: Fix Bug fix, typo fix, or any other fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants