-
Notifications
You must be signed in to change notification settings - Fork 278
Symfony 7.4 upgrade #3240
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: main
Are you sure you want to change the base?
Symfony 7.4 upgrade #3240
Conversation
9ebfa38 to
8479fcd
Compare
9ec2280 to
3cbf43e
Compare
…ible with symfony/cache anymore Since we don't use or need Redis, this is fine
3c8992b to
3fe9239
Compare
| PHPVERSION=$(php -r 'echo PHP_MAJOR_VERSION.".".PHP_MINOR_VERSION."\n";') | ||
| export PHPVERSION | ||
| echo "$PHPVERSION" | tee -a "$ARTIFACTS"/phpversion.txt | ||
| sudo apt-get -y purge "php$PHPVERSION-redis" |
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.
add a comment why this is required
| -name "bundles" -prune -o \ | ||
| -name "cache" -type d -prune -o \ | ||
| -name "ace" -type d -prune -o \ | ||
| -path "./webapp/config/reference.php" -prune -o \ |
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.
What is this and why are we doing this?
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.
See https://symfony.com/blog/new-in-symfony-7-4-better-php-configuration, it is an auto generated file for PHPstan/IDE basically to get code completion in the new format Symfony is moving towards (but we can't use yet). It will be generated everytime and they strongly advice to NOT put it in gitignore.
| - migrations | ||
| - tests | ||
| excludePaths: | ||
| - src/Utils/Adminer.php |
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 are you deleting this?
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 will add a comment in the new place, but basically PHPStan got very angry with how we did it before since it can't load adminer.
| if ($numCases > 0) { | ||
| $messages['info'][] = sprintf("Added/updated %d %s testcase(s): {%s}.{in,ans}", | ||
| $numCases, $type, join(',', $testcaseNames)); | ||
| $numCases, $type, implode(',', $testcaseNames)); |
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.
join is the much better name for the function, is it going away?
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.
https://www.php.net/join not really, will remove this rule.
| { | ||
| public function process(ContainerBuilder $container): void | ||
| { | ||
| if (PHP_VERSION_ID < 80400) { |
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.
Add a comment?
No description provided.