Skip to content

Widen PHP support #10

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

Merged
merged 2 commits into from
Oct 1, 2024
Merged

Widen PHP support #10

merged 2 commits into from
Oct 1, 2024

Conversation

juliangut
Copy link
Contributor

I know PHP 7.2 is already EOL but hear me out

This PR widens PHP support for the formatter to match requested PHPStan version 1.12.4, which supports PHP ^7.2|^8.0
The updates are the absolute minimum necessary to allow PHP 7.2 support, no behaviour is altered, added or removed

Given supported PHPStan version supports 7.2 I think this PR could be a reasonable change

@ruudk
Copy link
Member

ruudk commented Oct 1, 2024

Could you setup a CI matrix that tests all versions?

At least 7.2 and 8.3. That might be enough.

@ruudk
Copy link
Member

ruudk commented Oct 1, 2024

Thanks.

Do you think it would be good to also include a unit test? If you're up to it that would be great. 🙌

@ruudk ruudk merged commit 7532a89 into TicketSwap:main Oct 1, 2024
7 checks passed
@ruudk
Copy link
Member

ruudk commented Oct 1, 2024

@juliangut would you be up for that?

@juliangut
Copy link
Contributor Author

sorry, at this moment I was interested only on the PHP version support, don't think I have the time to create a test suite but I think it's a good idea of course

ruudk added a commit that referenced this pull request Oct 2, 2024
ruudk added a commit that referenced this pull request Oct 2, 2024
* Revert "Widen PHP support (#10)"

This reverts commit 7532a89.

* Lower PHP requirement to 8.1

PHPStan 2.0 is in the works, and it will only support 8.1 and higher.

* Run PHPStan on 8.1 (lowest)

* Move readonly to class properties
ruudk added a commit that referenced this pull request Oct 3, 2024
This broke in #10. The condition was wrong.

Added tests to assure this won't break again.
ruudk added a commit that referenced this pull request Oct 3, 2024
This broke in #10. The condition was wrong.

Added tests to assure this won't break again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants