Skip to content

Conversation

@nickygerritsen
Copy link
Member

No description provided.

@nickygerritsen nickygerritsen requested a review from vmcj November 28, 2025 12:08
@nickygerritsen nickygerritsen force-pushed the symfony-7.4 branch 2 times, most recently from 9ebfa38 to 8479fcd Compare November 28, 2025 12:14
@nickygerritsen nickygerritsen changed the title Minimum changes required for Symfony 7.4 upgrade Symfony 7.4 upgrade Nov 28, 2025
@Kevinjil Kevinjil marked this pull request as draft November 28, 2025 12:39
@nickygerritsen nickygerritsen force-pushed the symfony-7.4 branch 3 times, most recently from 9ec2280 to 3cbf43e Compare November 28, 2025 15:12
@nickygerritsen nickygerritsen marked this pull request as ready for review November 28, 2025 15:34
PHPVERSION=$(php -r 'echo PHP_MAJOR_VERSION.".".PHP_MINOR_VERSION."\n";')
export PHPVERSION
echo "$PHPVERSION" | tee -a "$ARTIFACTS"/phpversion.txt
sudo apt-get -y remove "php$PHPVERSION-redis"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably purge it in this case?

vmcj added a commit to vmcj/domjudge-packaging that referenced this pull request Nov 30, 2025
vmcj added a commit to vmcj/domjudge-packaging that referenced this pull request Nov 30, 2025
vmcj added a commit to vmcj/domjudge-packaging that referenced this pull request Nov 30, 2025
github-merge-queue bot pushed a commit to DOMjudge/domjudge-packaging that referenced this pull request Nov 30, 2025
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"
Copy link
Member

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 \
Copy link
Member

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?

Copy link
Member Author

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
Copy link
Member

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?

Copy link
Member Author

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));
Copy link
Member

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?

Copy link
Member Author

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.

export version="$1"
db=${2:-install}
phpversion="${3:-8.1}"
phpversion="${3:-8.2}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps move this commit (and I think there were other similar changes in other commits) to a separate PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

{
public function process(ContainerBuilder $container): void
{
if (PHP_VERSION_ID < 80400) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants