PHP 8.1 + Symfony 6.0 #675

ruudk · 2021-11-30T12:55:40Z

Fixes:

ruudk · 2021-11-30T13:38:22Z

@mihaileu It's ready :) I had to drop the older PHP versions to be able to upgrade to the latest PHPUnit version. Since they are not supported anyway, I think this is good.

javer · 2021-12-14T11:15:46Z

@ruudk Please resolve conflicts, and I hope this PR will be merged soon.

Drop all unsupported PHP and Symfony versions. Bump Ubuntu Drop stability and experimental

ruudk · 2021-12-14T11:25:16Z

@javer done

javer · 2021-12-14T11:28:27Z

@mihaileu Could you please review this PR?

Eloar · 2021-12-17T07:42:11Z

What's the point exactly? Is there any problem with using this bundle with newer Symfony or PHP version without those changes? So far bundle works with older versions and doesn't force users to upgrade whole projects just because this Bundle updated.

Also it is very good practice to squash commits for PR.

ruudk · 2021-12-17T07:44:29Z

What's the point exactly? Is there any problem with using this bundle with newer Symfony or PHP version without those changes? So far bundle works with older versions and doesn't force users to upgrade whole projects just because this Bundle updated.

I wouldn't have done this work if it wasn't needed obviously. There is no point in keeping this bundle in par with unsupported PHP versions. People on those versions should upgrade or keep the older releases of this bundle.

Also it is very good practice to squash commits for PR.

That's your opinion. That's not how I work.

javer · 2021-12-17T07:55:19Z

@Eloar Of course it cannot be used with newer Symfony version, because it cannot even be installed at all:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - php-amqplib/rabbitmq-bundle[2.10.0, ..., 2.10.2] require symfony/dependency-injection ^4.3|^5.0 -> found symfony/dependency-injection[v4.3.0, ..., v4.4.34, v5.0.0, ..., v5.4.1] but the package is fixed to v6.0.1 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
    - Root composer.json requires php-amqplib/rabbitmq-bundle 2.10.* -> satisfiable by php-amqplib/rabbitmq-bundle[2.10.0, 2.10.1, 2.10.2].

That's how composer constraints work:

RabbitMqBundle/composer.json

Lines 13 to 21 in 777bd90

    
           "symfony/dependency-injection": "^4.3|^5.0", 
        
           "symfony/event-dispatcher":     "^4.3|^5.0", 
        
           "symfony/config":               "^4.3|^5.0", 
        
           "symfony/yaml":                 "^4.3|^5.0", 
        
           "symfony/console":              "^4.3|^5.0", 
        
           "php-amqplib/php-amqplib":      "^2.12.2|^3.0", 
        
           "psr/log":                      "^1.0 || ^2.0 || ^3.0", 
        
           "symfony/http-kernel":          "^4.4|^5.0", 
        
           "symfony/framework-bundle":     "^4.4|^5.0"

javer · 2021-12-17T08:06:20Z

Can confirm that proposed changes works great in the real project with Symfony 6.0 and PHP 8.1.

Eloar · 2021-12-17T08:47:54Z

So there is no point for dropping support, just extending it for new Symfony version. Old Symfony versions support is not a problem, the problem is lack of Symfony6 support.

I would recommend not changing lower versions constraints until it became problem for bundle. It is not up to a bundle to enforce people to upgrade projects.

ruudk · 2021-12-17T08:51:22Z

@Eloar Feel free to create a PR and do it your way.

Eloar · 2021-12-17T09:16:45Z

@ruudk to conflict changes in active Pull Request? That would be counterproductive. Decision is up to Bundle owner I just shared opinion. I like how liberal this bundle is.

javer · 2021-12-17T09:26:58Z

PHPUnit 9.3+ is required to support PHP 8.0, and PHPUnit 9.5 is actually required to support PHP 8.1.
PHPUnit 9.5 requires php >=7.3, so we have to drop at least 7.1 and 7.2 which are beyond their EOL for at least one year.

Eloar · 2021-12-17T09:37:58Z

@javer is there a point to drop PHPUnit ^7.5|^8.5 version constraint and not just extend it with ^9.5 for PHP 8.1?

mihaileu · 2021-12-17T12:54:33Z

@ruudk @javer thx for this PR
I'll take a peek in this PR. For us it's important to keep the compatibility with the current allowed sy version in order to not create a BC and increase major. Otherwise we will need to maintain 2 versions of this library. I'll look into to see if we cover this situation

ramunasd · 2021-12-21T08:54:15Z

Regarding PHPUnit compatibility, You can use same workaround as we did here php-amqplib/php-amqplib@393546c#diff-96a82592013e5f4b91e682332583b7a4a6ca1ff0431d267c61179b1a21b167daR6

ruudk · 2021-12-21T08:58:12Z

Feel free to use my PR and improve it further. I'm not going to change it and support PHP versions that are EOL. Sorry. It took me already way too much time.

mihaileu · 2021-12-17T13:04:32Z

composer.json

-        "symfony/config":               "^4.3|^5.0",
-        "symfony/yaml":                 "^4.3|^5.0",
-        "symfony/console":              "^4.3|^5.0",
+        "symfony/dependency-injection": "^4.4|^5.3|^6.0",


why ^5.3, isn't it working with older versions of 5 ?

https://symfony.com/releases , only 5.3 is supported, the rest is EOL

@ramunasd I see no reason to let old compatibility with php and symfony EOL. We will keep same major tag an increase the minor. There's kinda enough time passed in order to not affect too much people or at lease suggest them an update. What do you think ?

AFAIK symfony 4.4 and 5.4 are LTS releases, we can stick to them

mihaileu · 2021-12-17T13:06:31Z

DependencyInjection/OldSoundRabbitMqExtension.php

@@ -654,7 +654,7 @@ protected function injectConnection(Definition $definition, $connectionName)
        $definition->addArgument(new Reference(sprintf('old_sound_rabbit_mq.connection.%s', $connectionName)));
    }

-    public function getAlias()
+    public function getAlias() : string


is there a space between ) and : ? if yes, we should eliminate that in order to stick as much as possbile with PSR12

php-cs-fixer should do this.

Let's add PSR12 style check for all PR's. These things should be noticeable automatically.

i'll add php-cs-fixer as github action

ruudk · 2021-12-21T13:13:15Z

Didn't expect this to be merged, thank you! @mihaileu 🎉

mihaileu · 2021-12-21T13:18:53Z

thanks for your contribution

ruudk force-pushed the patch-7 branch 4 times, most recently from 384fe3d to 0c65ea5 Compare November 30, 2021 13:36

ruudk marked this pull request as ready for review November 30, 2021 13:36

ruudk requested a review from mihaileu as a code owner November 30, 2021 13:36

ruudk mentioned this pull request Nov 30, 2021

Drop Symfony 5.3 and add support for Symfony 6.0 SimpleBus/SimpleBus#149

Merged

ruudk added 11 commits December 14, 2021 12:21

Change stability from RC to stable

25bd4fd

Test on PHP 8.1

529aba6

Test Symfony 6.0 on PHP 8.0 and 8.1

c15e28f

Allow Symfony 6.0

11f572a

Add missing return type

a7eb79e

Bump minimum required PHP version to PHP 7.4

f52f86a

Bump minimum required Symfony version to 4.4, 5.3, 6.0

eb671a3

Bump PHPUnit to 9.5

1161d15

Update workflow

17b2d4d

Drop all unsupported PHP and Symfony versions. Bump Ubuntu Drop stability and experimental

Bump PHPStan to 1.2

bfbce72

Pin Symfony versions using Flex

30175c8

ruudk force-pushed the patch-7 branch from 67ea333 to 30175c8 Compare December 14, 2021 11:21

javer approved these changes Dec 17, 2021

View reviewed changes

mihaileu mentioned this pull request Dec 17, 2021

Symfony 5.4 deprecations adressed #679

Merged

javer mentioned this pull request Dec 21, 2021

Add Symfony 6 support #680

Closed

mihaileu reviewed Dec 21, 2021

View reviewed changes

mihaileu merged commit 638ea41 into php-amqplib:master Dec 21, 2021

ruudk deleted the patch-7 branch December 21, 2021 12:58

This was referenced Dec 21, 2021

Drop unsupported Symfony versions #677

Closed

Drop unsupported PHP versions #676

Closed

PHP 8.1 + Symfony 6.0 #675

PHP 8.1 + Symfony 6.0 #675

Uh oh!

Conversation

ruudk commented Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruudk commented Nov 30, 2021

Uh oh!

javer commented Dec 14, 2021

Uh oh!

ruudk commented Dec 14, 2021

Uh oh!

javer commented Dec 14, 2021

Uh oh!

Eloar commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruudk commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

javer commented Dec 17, 2021

Uh oh!

javer commented Dec 17, 2021

Uh oh!

Eloar commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruudk commented Dec 17, 2021

Uh oh!

Eloar commented Dec 17, 2021

Uh oh!

javer commented Dec 17, 2021

Uh oh!

Eloar commented Dec 17, 2021

Uh oh!

mihaileu commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ramunasd commented Dec 21, 2021

Uh oh!

ruudk commented Dec 21, 2021

Uh oh!

mihaileu Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

ruudk Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mihaileu Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

ramunasd Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

mihaileu Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

ruudk Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

ramunasd Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

mihaileu Dec 21, 2021

Choose a reason for hiding this comment

Uh oh!

ruudk commented Dec 21, 2021

Uh oh!

mihaileu commented Dec 21, 2021

Uh oh!

Uh oh!

ruudk commented Nov 30, 2021 •

edited

Loading

Eloar commented Dec 17, 2021 •

edited

Loading

ruudk commented Dec 17, 2021 •

edited

Loading

Eloar commented Dec 17, 2021 •

edited

Loading

mihaileu commented Dec 17, 2021 •

edited

Loading

ruudk Dec 21, 2021 •

edited

Loading