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

Couple of fixes in QuickForm2 for PHP8.1 compatibility #18309

Merged
merged 2 commits into from Nov 15, 2021
Merged

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 14, 2021

Description:

fixes #18117

Note: QuickForm2 would meanwhile also be available via composer, so we could switch using that instead. Unfortunately QuickForm2 is compatible with PHP 5.4+, which makes it impossible to have it compatible with PHP 8.1
as well. I first created an approach using composer and forked the QuickForm2 repos to adjust the fixes (see https://github.com/matomo-org/matomo/compare/quickform_composer), but guess they might not be merged upstream because of the PHP requirement maybe. So guess simply adjusting the code here would be the simplest solution for now.

Review

@sgiehl sgiehl added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Nov 14, 2021
@sgiehl sgiehl added this to the 4.6.0 milestone Nov 14, 2021
Copy link
Member

@tsteur tsteur left a comment

Choose a reason for hiding this comment

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

@sgiehl was it required to add the return types to the methods to fix the PHP 8.1 issue? Just wondering if this might cause a regression (it's a bit of work checking the signature doesn't change and cause issues)

@sgiehl
Copy link
Member Author

sgiehl commented Nov 15, 2021

Yes kind of. The other solution would be to add a #[\ReturnTypeWillChange] attribute to those methods.
But I check each page were I needed to apply a fix with PHP 8.1 and PHP 7.2, to ensure it doesn't regress for older versions

@tsteur tsteur merged commit c4431ba into 4.x-dev Nov 15, 2021
@tsteur tsteur deleted the quickform_php81 branch November 15, 2021 23:02
@justinvelluppillai justinvelluppillai removed the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Matomo 4.5 not working with PHP 8.1 RC3 (Matomo 4.4.1 worked fine)
3 participants