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.

Sign up for GitHub

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

Jump to bottom

[PHP 8.4] Fixes for implicit nullability deprecation #154

Merged
merged 2 commits into from
Mar 27, 2024
Merged

[PHP 8.4] Fixes for implicit nullability deprecation #154

merged 2 commits into from
Mar 27, 2024

Conversation

Ayesh
Copy link

@Ayesh Ayesh commented Mar 15, 2024

Fixes all issues that emit deprecation notices on PHP 8.4 for implicit nullable parameter type declarations.

See:

jrfnl
jrfnl reviewed Mar 15, 2024
View reviewed changes
Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

@Ayesh Thanks for the PR. Unfortunately, this PR cannot be accepted.

PHP-Parallel-Lint/composer.json

Line 13 in 539292f

"php": ">=5.3.0",

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 16, 2024 •
edited
Loading

@Ayesh Would you be okay with me updating this PR to get it into a mergable state ?

@grogy Once this patch is ready, we should merge it, cherrypick it to master and release it as soon as possible as people using PHP Parallel Lint to already check their code against PHP 8.4 will currently get failing builds due to this deprecation.

As develop and master have some differences at this time, I'll open a ticket in a moment with a proposal on how to deal with this and what's needed (in my opinion) to do a release.

@jrfnl jrfnl added this to the 1.3.x Next milestone Mar 16, 2024
@jrfnl jrfnl mentioned this pull request Mar 16, 2024
Closed
@Ayesh
Copy link
Author

Ayesh commented Mar 16, 2024

Please feel free to update the PR. If it makes things easier/cleaner, lets just close this one. Completely trusting you hands that you will sort balance the deprecation notices and the best compatibility.

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 16, 2024

Please feel free to update the PR. If it makes things easier/cleaner, lets just close this one. Completely trusting you hands that you will sort balance the deprecation notices and the best compatibility.

Thanks @Ayesh ! I've just pushed an update. Let's hope the build passes ;-)

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 16, 2024

I've just cherrypicked the commit from PR #152 into this one to get the builds running properly. 🤞🏻

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 16, 2024

Test failure on PHP 8.4 looks to be related to incompatibilities in the last version of PHPUnit 9 (will likely be fixed soon), not related to this package itself.

@jrfnl
Copy link
Collaborator

jrfnl commented Mar 16, 2024

I've done a test run which combines the changes from PR #152, #157 and this PR and that gets us a passing build on PHP 8.4, which confirms that this patch fixes the PHP 8.4 issue.

https://github.com/php-parallel-lint/PHP-Parallel-Lint/actions/runs/8310177747

jrfnl
jrfnl approved these changes Mar 16, 2024
View reviewed changes
Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Thanks @Ayesh for the PR and starting the conversation about fixing this deprecation.

Patch as-is is good to go and has been confirmed as working cross-version (see my previous comment).

Waiting to merge as PRs #152 and #158 will need to be merged first to get a passing build.

@Ayesh
Copy link
Author

Ayesh commented Mar 16, 2024

This is amazing, thank you!

@grogy
Copy link
Member

grogy commented Mar 27, 2024

Thank to both, it looks great. I merge it.

@grogy grogy merged commit db896de into php-parallel-lint:develop Mar 27, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrfnl jrfnl jrfnl approved these changes

Assignees
No one assigned
Labels
Type: chores/QA
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Ayesh @jrfnl @grogy