-
Notifications
You must be signed in to change notification settings - Fork 193
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
feat: drop support of php 7.4 #376
feat: drop support of php 7.4 #376
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.
Sorry I didn't notice this PR before proposing mine, see #378
I'm inclined to accept yours and close mine, but do you mind to take a look and take some changes from it?
I merged your changes, except from the static data providers, as i made a seperate PR for that. #377 I think the php 8.1 test-run should be on symfony 6.2 as 6.1 is not maintained anymore. https://symfony.com/releases/6.1 |
.github/workflows/build.yaml
Outdated
symfony: 5.4.* | ||
- description: 'Symfony 6.0' | ||
php: '8.0' | ||
symfony: 6.0.* | ||
- description: 'Dev deps' | ||
- description: 'Symfony 6.1' |
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.
To me, we should have jobs for a specific Symfony version only for LTS 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.
… or at least supported branches only. Symfony 6.0 and 6.1 are dead.
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.
We should test all the Symfony versions we declare support for
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.
all the Symfony versions we declare support for
Which are?
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.
the ones resolved in our composer.json: ^5.4 || ^6.0
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.
Why should we test each minor version of Symfony while not doing the same for other dependencies (Twig).
To me, we should be pragmatic here, having specific jobs only for LTS versions, which would avoid having to update the CI setup every 6 months.
The jobs running with unmodified dependencies already test with the latest stable Symfony version anyway.
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.
OK then, let's go with the LTS versions only
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 hope i did the changes correct like suggested. Please have a look.
.github/workflows/build.yaml
Outdated
symfony: 6.0.* | ||
- description: 'Dev deps' | ||
symfony: 5.4.* | ||
- description: 'Symfony 5.4' |
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.
We don't need multiple jobs for Symfony 5.4
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 changed it, can you please have a look if it now ok?
#374
Probably more could be changed. But i tried to not introduce any BC changes.
I am not sure how to handle the --prefer-lowest failing run (indirect deprecation notices from twig 1). https://github.com/Chris53897/KnpMenu/actions/runs/4971653642/jobs/8896326293
https://symfony.com/doc/current/components/phpunit_bridge.html#direct-and-indirect-deprecations