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

Make adjustments to support PHP8.1's stricter type system #2695

Merged
merged 7 commits into from
Feb 22, 2022
Merged

Conversation

nezaniel
Copy link
Member

@nezaniel nezaniel commented Feb 6, 2022

This adds additional type hints where required in PHP 8.1 while maintaining support for 7.4.
These are only the adjustments required fir the ESCR test suite, more are to follow

Resolves: #2698

@mficzel
Copy link
Member

mficzel commented Feb 9, 2022

It would be great to add php 8.1 to the test matrix in .github/workflows/build.yml together with this.

Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

Looks good by reading, but I agree having 8.1 builds run would be good

@nezaniel
Copy link
Member Author

8.1 builds are configured and run through smoothly, which leaves me a bit sceptical since that seems improbable. Is there more to be configured or is everything fine already?

@albe
Copy link
Member

albe commented Feb 15, 2022

8.1 builds are configured and run through smoothly, which leaves me a bit sceptical since that seems improbable. Is there more to be configured or is everything fine already?

==> Setup PHP
✓ PHP Found PHP 8.1.2

Looks good to me. Are there any specific reasons why you think running Flow on 8.1 should not work with this PR applied @nezaniel ? Are there any breaking changes in 8.1 that we should be subject to?
I could very well imagine Flow being okay and Neos having a few more edge cases from a gut feeling

@mficzel
Copy link
Member

mficzel commented Feb 15, 2022

i somehow miss the psalm test in the matrix now. Should offcourse not run on 7.3 but on 8.0 i think. Or was it disabled for not behaving well.

@nezaniel
Copy link
Member Author

@albe Well, I had to un-proxy literally every ESCR exception class for it to not throw a __wakeup() signature notice, so I expected this to happen all over the Flow code base... but maybe the ESCR test setup is a little more strict than the regular tests and PHP runtimes

As far as I could see, the psalm tests were configured for 7.3 only, so I disabled them

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

Looking good in general, I just added to nitpicky comments

…Trait.php

Co-authored-by: Bastian Waidelich <bastian@neos.io>
@nezaniel nezaniel merged commit 19ffef8 into master Feb 22, 2022
@nezaniel nezaniel deleted the php8.1 branch February 22, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust Flow for PHP 8.1
4 participants