-
-
Notifications
You must be signed in to change notification settings - Fork 0
Update PHP requirements and improve code annotations #3
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
base: master
Are you sure you want to change the base?
Conversation
We are running under PHP 8.x by default but still support PHP 7.4. Psalm would assume that we need PHP 8.x features overwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As usual, nit-picks.
On the pipeline errors:
- While psalm failure is expected in php 8.0 and 8.1, it should run in 7.4.
- The require-checker is failing in lower versions too.
Triggering the pipeline again pulled the correct psalm versions but now the psalm.xml is incompatible. Psalm 5.x does not know the Sidenote: PHP7.4 does phpcq install instead of update and therefore pulls the tools for current master - this seems to be wrong |
I have no idea how to fix it properly. |
The only idea I have is providing separate config files for different conflicting php versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go - however the pipeline failures should get adressed by either:
- fixing them (what ever the way to go might be then)
- dropping support for end of life versions (which will break these then).
I vote for dropping support for end of life PHP versions. We would still have to support PHP 8.1. There is a broken psalm version used build against PHP 8.2. Haven't you release the version override for psalm already? |
Yes, the overrides are in place and working. |
This pull request includes the following changes: