-
Notifications
You must be signed in to change notification settings - Fork 946
Fix PHP 8 compatibility in 2.x #882
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
Conversation
it would be very hard to test with php 5.6 until php 8 due to phpunit bc breaks and incompatibilities. so just droppping unsupported php versions is the best choice which was also done in master 3.x (master). This backports zircote#830, zircote#837, zircote@29e913a, parts of zircote@3f1cee9 to fix php 8 errors and tests.
@DerManoMann tests pass. I would appreciate if you tag this as 2.1 release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work - thanks.
}, | ||
"autoload-dev": { | ||
"psr-4": { | ||
"SwaggerTests\\": "tests/" | ||
"SwaggerTests\\": "tests/", | ||
"AnotherNamespace\\": "tests/Fixtures/AnotherNamespace" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noit sure if this is really needed seeing that the fixtures do not properly use PSR-4 namespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fixtures use psr-4 namespace autoloading corrctly. Just with strange namespaces (\SwaggerTests, \AnotherNamespace). So I think this is needed to load the annotation examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I would argue that
namespace SwaggerFixtures;
is not a correct namespace according to PSR-0/PSR-4 as it does not match the folder name at all.
Nevertheless, that's not really important for the fixtures so all is good.
@DerManoMann can't you create the tag? |
I'll double check - looks like I can... I always mix up GH and packagist permissions :/ |
@DerManoMann thanks for the tag. Unfortunately the release is not visible to packagist: https://packagist.org/packages/zircote/swagger-php. Can you trigger a refresh? |
damn, got distracted! autoupdate is definitely something I cannot configure |
My library fails with php 8 using swagger 2.x, see Tobion/OpenAPI-Symfony-Routing#7
This is due to the namespace not being parsed correctly.
This PR fixes PHP 8 compatibility.
It would be very hard to test with php 5.6 until php 8 due to phpunit bc breaks and incompatibilities, e.g. phpunit <9 will not support php 8. so just droppping unsupported php versions is the best choice which was also done in 3.x (master).
This backports #830, #837, 29e913a, parts of 3f1cee9 to fix php 8 errors and tests.