Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(psalm): Enforce that psalm is executed against the minimum PHP version #466

Merged

Conversation

nickvergessen
Copy link
Member

As discussed the other day with @st3iny

@st3iny

Sounds good, but psalm will probably take some more time: vimeo/psalm#11107

@nickvergessen

https://github.com/nextcloud/spreed/actions/runs/11930485130/job/33251359582 talk psalm works on 8.4
it doesnt have a hardcoded version check breaking it
might of course depend on edge cases your code triggers
but I would think Talk is pretty edgy

@st3iny

There are some deprecation warnings and it broke in one of my maintained apps though
Can't remember which one

@nickvergessen

the other fix for psalm is making sure you have the correct base version
https://github.com/nextcloud/spreed/blob/main/psalm.xml#L8
otherwise it checks your code to only be compatible with the actually used php version
instead of the minimum
we should add a grep call to the ci workflow, to make sure the phpversion is added

psalm.xml

psalm-matrix.xml

…rsion

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added 3. to review Waiting for reviews bug Something isn't working labels Nov 27, 2024
@nickvergessen nickvergessen requested a review from st3iny November 27, 2024 20:49
@nickvergessen nickvergessen self-assigned this Nov 27, 2024
@nickvergessen nickvergessen merged commit de1c740 into master Nov 27, 2024
5 checks passed
@nickvergessen nickvergessen deleted the bugfix/noid/enforce-psalm-runs-against-php-min branch November 27, 2024 20:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants