-
-
Notifications
You must be signed in to change notification settings - Fork 154
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
Bump minimum PHP version required to PHP 8.1 #1765
Conversation
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.
Could you please create a PR on our docs to add this new requirement?
See https://github.com/infection/site/blob/master/src/guide/installation.md
.github/workflows/ci.yaml
Outdated
coverage-driver: [ pcov, xdebug ] | ||
e2e-runner: [ 'bin/infection' ] | ||
include: | ||
- { operating-system: 'windows-latest', php-version: '8.0', coverage-driver: 'xdebug', e2e-runner: 'bin/infection' } | ||
- { operating-system: 'ubuntu-latest', php-version: '8.0', coverage-driver: 'pcov', e2e-runner: 'build/infection.phar' } | ||
- { operating-system: 'ubuntu-latest', php-version: '8.0', coverage-driver: 'xdebug', e2e-runner: 'build/infection.phar' } |
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.
Looks like we need to add to the matrix PHP 8.1 with xdebug
and build/infection.phar
?
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.
Fixed; I didn't realise one was for windows.
I'm a bit confused by the matrix though, we have pcov | xdebug
for code coverage driver and bin/infection & build/infection.phar
for the runner but only 2 combinations for 8.0 on ubuntu, what was the reasoning there?
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.
I guess to save time, as we didn't want to check all the combinations
Done: infection/site#250 |
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.
Looks good (except the failing build)
6aebfc6
to
63d2c5c
Compare
@theofidry what is now blocking this PR? |
@maks-rafalko CI not green and a lot of changes. One thing that sprout from this was that it was running Symfony6 I think, which was not working although we declared Symfony6 compatibility. So to move forward, I think #1824 is required, then I'll be able to rebase this and see what's left |
Closes #1760
PR doc: infection/site#250