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.4 #98

Open
wants to merge 2 commits into
base: 3.15.x
Choose a base branch
from

Conversation

tux-rampage
Copy link
Member

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

Description

This will add support for PHP 8.4

Signed-off-by: tux-rampage <[email protected]>
@tux-rampage tux-rampage added this to the 3.15.0 milestone Dec 9, 2024
@tux-rampage tux-rampage self-assigned this Dec 9, 2024
@tux-rampage
Copy link
Member Author

Interesting, the tooling used in the GitHub actions seems to be a bit out of date. They use 8.4.0RC4 and the composer.phar throws implicit null Notices.

Locally a composer update on the project with 8.4 worked without any issues.

@tux-rampage tux-rampage requested a review from a team December 9, 2024 10:12
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to ignore platform reqs on 8.4 to get dependencies installed: https://github.com/laminas/laminas-filter/pull/181/files

I think the CI matrix action will need a new release to use the 8.4 GA release, but the RC should be fine for the time being.

Most risky/truthy Psalm issues can be solved by changing conditionals from if (! $thing) to if ($thing !== null) or $this->thing = $thing ?? 'default', or revert the lockfile and composer update nothing so the psalm issues can be dealt with in a separate patch maybe

@tux-rampage
Copy link
Member Author

You'll need to ignore platform reqs on 8.4 to get dependencies installed: https://github.com/laminas/laminas-filter/pull/181/files

Thanks for the Tipp. Will check this.

Most risky/truthy Psalm issues can be solved by changing conditionals from if (! $thing) to if ($thing !== null) or $this->thing = $thing ?? 'default', or revert the lockfile and composer update nothing so the psalm issues can be dealt with in a separate patch maybe

This check is failing for a while, but addressing it here would be out of scope and require a separate PR.

Signed-off-by: tux-rampage <[email protected]>
@tux-rampage
Copy link
Member Author

PHPUnit for 8.4 latest runs sucessfull now, 8.4 lowest does not.

It seems to be the lowest allowed PHPUnit version. Can we raise it to ^9.6.22 for now? This should fix it.

@gsteel
Copy link
Member

gsteel commented Dec 13, 2024

Yeah - do what you want with dev deps. I always run composer bump -D with abandon.

You may also need to bump laminas-stdlib in require to an 8.4 compatible release because of implicit nulls there.

@tux-rampage tux-rampage mentioned this pull request Jan 7, 2025
@tux-rampage tux-rampage linked an issue Jan 7, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

php 8.4 support
2 participants