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 support for PHP 8.3 #49

Merged
merged 4 commits into from
Oct 1, 2023
Merged

Add support for PHP 8.3 #49

merged 4 commits into from
Oct 1, 2023

Conversation

MauricioFauth
Copy link
Contributor

Q A
New Feature yes

Description

Adds support for PHP 8.3.

@MauricioFauth
Copy link
Contributor Author

Looks like the php8.3-igbinary and php8.3-msgpack extensions are not available yet. Since those extensions are optional, shouldn't the CI test with and without them?

@boesing
Copy link
Member

boesing commented Oct 1, 2023

Since those extensions are optional, shouldn't the CI test with and without them?

They are optional, indeed.
But they're optional for upstream projects. IMHO required for CI since shipping stuff without proper support for optional stuff is not how it should work.
We might be able to install extensions via PECL.

Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
@boesing boesing added this to the 2.17.0 milestone Oct 1, 2023
@boesing boesing self-assigned this Oct 1, 2023
@MauricioFauth
Copy link
Contributor Author

But they're optional for upstream projects. IMHO required for CI since shipping stuff without proper support for optional stuff is not how it should work.

Yeah, I agree with testing with the extensions. I meant to also test without the extensions to see if the rest of the library properly works when the optional extensions are not installed.

@boesing
Copy link
Member

boesing commented Oct 1, 2023

Thats definitely something we could do, but that would most likely require to re-configure the checks made by this project.
Not sure if its worth the hassle as it would be almost impossible to determine which extensions are required and which are optional (ofc. one could check the composer.json).

Might be more beneficial if we simply ignore uninstallable extensions instead of failing hard. Can you create a feature request in the CI container action? That would be helpful, I'll manage to make the tests pass.
Already on a fix for the pecl issue in laminas/laminas-continuous-integration-action#189

sury does not (yet) provide those extensions and thus we have to install them via PECL as a fallback.

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Ref: https://wiki.php.net/rfc/unserialize_warn_on_trailing_data
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing merged commit 9641dee into laminas:2.17.x Oct 1, 2023
15 checks passed
@boesing
Copy link
Member

boesing commented Oct 1, 2023

Thanks, @MauricioFauth!

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.

2 participants