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

Add PHP 8.3 support #47

Merged
merged 5 commits into from
Dec 1, 2023
Merged

Conversation

GuySartorelli
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA no

Description

PHP 8.3 can't be used with this library due to the strict dependency constraint.
Loosening the constraint to allow new versions of PHP 8 to be automatically installable isn't allowed as per #46 (comment), so I'm just adding PHP 8.3 support here.

I've checked against the changes listed in https://php.watch/versions/8.3 and none of the listed changes should break anything this library uses.

Hopefully your CI will automagically pick up that it needs to test against PHP 8.3 - but if not, please let me know what I need to change and I'll change it.

Closes #46

@Xerkus
Copy link
Member

Xerkus commented Nov 23, 2023

php 8.0 reached eol. Please also drop it and update platform to 8.1

@GuySartorelli GuySartorelli mentioned this pull request Nov 23, 2023
@Xerkus Xerkus added this to the 2.18.0 milestone Nov 23, 2023
@GuySartorelli
Copy link
Contributor Author

.......... Okay. That's not at all in the scope of what I'm doing this change for, but if that's what has to be done for you to add PHP 8.3 support, I'll do it.

@GuySartorelli
Copy link
Contributor Author

Done

@GuySartorelli
Copy link
Contributor Author

GuySartorelli commented Nov 23, 2023

Looks like there are some PHP 8.1 failures in CI - those are out of scope for this PR.

@Xerkus
Copy link
Member

Xerkus commented Nov 23, 2023

Those errors appear to be more than just from updates to static analysis tool. They would need to be addressed first before this PR can be merged.
I'll take a look at it later.

Signed-off-by: Aleksei Khudiakov <[email protected]>
Signed-off-by: Aleksei Khudiakov <[email protected]>
Signed-off-by: Aleksei Khudiakov <[email protected]>
@Xerkus Xerkus force-pushed the pulls/2.18.x/php-83 branch from 39252a8 to c8d6d19 Compare December 1, 2023 20:13
@Xerkus
Copy link
Member

Xerkus commented Dec 1, 2023

I do not understand why code sniffer invokes fatal error requiring return type will change annotation but it does not fail locally on php 8.2 or via tests

@Xerkus
Copy link
Member

Xerkus commented Dec 1, 2023

Hm. That is a LOT of skipped tests

Signed-off-by: Aleksei Khudiakov <[email protected]>
@Xerkus
Copy link
Member

Xerkus commented Dec 1, 2023

generic ldap resource changed to dedicated objects starting with php 8.1. Since php 8.1 is now minimum the resource type is no longer valid so phpdoc was updated to reflect that and checks for resource were simplified to look only for relevant classes.

Missing ReturnTypeWillChange annotation indicates that support for php 8.1 and 8.2 was incomplete. It was not previously picked up most likely due to disabled online tests. That would be handled in #48

@Xerkus Xerkus merged commit 72b438e into laminas:2.18.x Dec 1, 2023
@GuySartorelli GuySartorelli deleted the pulls/2.18.x/php-83 branch December 1, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

PHP 8.3 support
3 participants