Skip to content
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

Upgrade to Symfony 5 #7137

Merged
merged 17 commits into from
Jan 25, 2024
Merged

Upgrade to Symfony 5 #7137

merged 17 commits into from
Jan 25, 2024

Conversation

yguedidi
Copy link
Contributor

@yguedidi yguedidi commented Dec 24, 2023

requires those PRs to be merged first:

better be reviewed commit by commit

Production Changes From To Compare
doctrine/annotations 1.14.3 2.0.1 ...
doctrine/doctrine-bundle 2.7.2 2.11.1 ...
doctrine/doctrine-migrations-bundle 3.2.5 3.3.0 ...
enshrined/svg-sanitize 0.15.4 0.16.0 ...
friendsofsymfony/rest-bundle 3.5.0 3.6.0 ...
incenteev/composer-parameter-handler v2.1.5 v2.2.0 ...
jms/serializer-bundle 5.3.1 5.4.0 ...
lcobucci/jwt 4.1.5 4.3.0 ...
lexik/form-filter-bundle v7.0.3 REMOVED
nelmio/api-doc-bundle v4.11.1 v4.18.1 ...
nelmio/cors-bundle 2.3.1 2.4.0 ...
phpdocumentor/type-resolver 1.7.3 1.8.0 ...
phpstan/phpdoc-parser 1.24.5 1.25.0 ...
psr/http-message 1.1 2.0 ...
stof/doctrine-extensions-bundle v1.7.2 v1.10.1 ...
symfony/asset v4.4.46 v5.4.31 ...
symfony/config v4.4.44 v5.4.31 ...
symfony/console v4.4.49 v5.4.34 ...
symfony/debug v4.4.44 REMOVED
symfony/dependency-injection v4.4.49 v5.4.34 ...
symfony/doctrine-bridge v4.4.48 v5.4.34 ...
symfony/dom-crawler v4.4.45 v5.4.32 ...
symfony/error-handler v4.4.44 v5.4.29 ...
symfony/event-dispatcher v4.4.44 v5.4.34 ...
symfony/event-dispatcher-contracts v1.10.0 v2.5.2 ...
symfony/finder v4.4.44 v5.4.27 ...
symfony/form v4.4.48 v5.4.33 ...
symfony/framework-bundle v4.4.51 v5.4.34 ...
symfony/google-mailer v4.4.41 v5.4.23 ...
symfony/http-foundation v4.4.49 v5.4.34 ...
symfony/http-kernel v4.4.51 v5.4.34 ...
symfony/mailer v4.4.49 v5.4.34 ...
symfony/mime v4.4.47 v5.4.26 ...
symfony/monolog-bridge v5.2.12 v5.4.31 ...
symfony/monolog-bundle v3.8.0 v3.10.0 ...
symfony/options-resolver v4.4.44 v5.4.21 ...
symfony/proxy-manager-bridge v4.4.39 v5.4.21 ...
symfony/psr-http-message-bridge v2.1.4 v2.3.1 ...
symfony/routing v4.4.44 v5.4.34 ...
symfony/security-bundle v4.4.50 v5.4.34 ...
symfony/security-core v4.4.48 v5.4.30 ...
symfony/security-csrf v5.2.12 v5.4.27 ...
symfony/security-guard v4.4.46 v5.4.27 ...
symfony/security-http v4.4.50 v5.4.31 ...
symfony/templating v4.4.44 v5.4.21 ...
symfony/translation v4.4.47 v5.4.31 ...
symfony/twig-bridge v4.4.51 v5.4.34 ...
symfony/twig-bundle v4.4.41 v5.4.31 ...
symfony/validator v4.4.48 v5.4.34 ...
symfony/var-dumper v4.4.47 v5.4.29 ...
symfony/yaml v5.3.14 v5.4.31 ...
twig/extra-bundle v3.7.0 v3.8.0 ...
zircote/swagger-php 4.8.2 4.8.3 ...
ezyang/htmlpurifier NEW v4.17.0
psr/event-dispatcher NEW 1.0.0
spiriitlabs/form-filter-bundle NEW v10.0.0
symfony/password-hasher NEW v5.4.31
symfony/polyfill-intl-icu NEW v1.28.0
Dev Changes From To Compare
composer/pcre 1.0.1 3.1.1 ...
composer/xdebug-handler 2.0.5 3.0.3 ...
dama/doctrine-test-bundle v7.1.1 v8.0.1 ...
doctrine/doctrine-fixtures-bundle 3.4.5 3.5.1 ...
friendsofphp/php-cs-fixer v3.4.0 v3.47.1 ...
php-cs-fixer/diff v2.0.2 REMOVED
phpstan/phpstan-doctrine 1.3.58 1.3.59 ...
phpunit/phpunit 9.6.15 9.6.16 ...
symfony/browser-kit v4.4.44 v5.4.31 ...
symfony/css-selector v4.4.44 v5.4.26 ...
symfony/debug-bundle v4.4.37 v5.4.26 ...
symfony/maker-bundle v1.39.1 v1.43.0 ...
symfony/phpunit-bridge v6.4.2 v7.0.2 ...
symfony/web-profiler-bundle v4.4.47 v5.4.34 ...

Comment on lines +64 to +65
['image/pjpeg', 'jpg'],
['image/jpeg', 'jpg'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think is safe and BC because it seems this is used only to generate new image files, so no impact on previously generated ones I think, if someone can confirm :)

@yguedidi
Copy link
Contributor Author

On my local the single failing test fails with the following error:

[2023-12-24T23:47:32.817187+00:00] request.CRITICAL: Uncaught PHP Exception PHPUnit\Framework\Error\Warning: "require(/var/www/html/var/cache/tes_/Container36H4AEP/RabbitMQConsumerTotalProxy_2d65184.php): Failed to open stream: No such file or directory" at /var/www/html/var/cache/tes_/Container36H4AEP/AppKernelTestDebugContainer.php line 892 {"exception":"[object] (PHPUnit\\Framework\\Error\\Warning(code: 2): require(/var/www/html/var/cache/tes_/Container36H4AEP/RabbitMQConsumerTotalProxy_2d65184.php): Failed to open stream: No such file or directory at /var/www/html/var/cache/tes_/Container36H4AEP/AppKernelTestDebugContainer.php:892)"} []

Will need to investigate how to resolve it.
Weird thing is that postgres tests works...

@yguedidi yguedidi added this to the 2.7.0 milestone Dec 25, 2023
@yguedidi yguedidi force-pushed the upgrade-to-symfony-5 branch 2 times, most recently from fdb0914 to 268b140 Compare December 25, 2023 23:13
@yguedidi
Copy link
Contributor Author

@j0k3r @Kdecherf @nicosomb found this on master https://github.com/wallabag/wallabag/blob/master/tests/Wallabag/CoreBundle/Command/InstallCommandTest.php#L221-L229
seems we already skip a test because of a similar issue...
the failing test here works on my local using docker and sqlite database.
should I skip the failing test on mysql and sqlite?

@yguedidi yguedidi force-pushed the upgrade-to-symfony-5 branch 9 times, most recently from 786f7ea to b9a1418 Compare January 1, 2024 20:34
@yguedidi
Copy link
Contributor Author

yguedidi commented Jan 1, 2024

@j0k3r @nicosomb @Kdecherf it looks like the remaining error was because RabbitMQConsumerTotalProxy being lazy...
Found in the doc of the bundle that lazy services are not supported

@yguedidi yguedidi force-pushed the upgrade-to-symfony-5 branch 2 times, most recently from 174b3c0 to 7294f43 Compare January 2, 2024 07:46
@yguedidi
Copy link
Contributor Author

yguedidi commented Jan 3, 2024

@j0k3r you can check this force-push diff for the fixes, I updated 220bcfd and 91c3f53

@yguedidi yguedidi mentioned this pull request Jan 3, 2024
2 tasks
@yguedidi yguedidi force-pushed the upgrade-to-symfony-5 branch 2 times, most recently from 22f9d60 to 383a9c3 Compare January 8, 2024 22:54
@yguedidi yguedidi marked this pull request as ready for review January 8, 2024 22:57
@yguedidi yguedidi requested review from nicosomb and Kdecherf and removed request for nicosomb and Kdecherf January 19, 2024 22:11
@yguedidi
Copy link
Contributor Author

Thanks @j0k3r ! @Kdecherf @nicosomb would be great if one of you could check this also, would prefer to have at least 2 approves before merging on this key PR :)

@yguedidi yguedidi mentioned this pull request Jan 22, 2024
1 task
Copy link
Member

@Kdecherf Kdecherf left a comment

Choose a reason for hiding this comment

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

Looks good

@Kdecherf Kdecherf merged commit eb36d69 into wallabag:master Jan 25, 2024
21 checks passed
@Kdecherf
Copy link
Member

Thanks @yguedidi 🎉

@yguedidi yguedidi deleted the upgrade-to-symfony-5 branch January 25, 2024 19:33
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.

None yet

3 participants