Updated to make it compatible with symfony 4.3

[macr1408]

Changed arguments order in the eventDispatcher in order to make it compatible with Symfony 4.3

Deprecation message:

php.INFO: User Deprecated: Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as first argument is deprecated since Symfony 4.3, pass it second and provide the event object first instead.

Q	A
Bug fix	yes
New feature	no
BC breaks	yes
Deprecations	no
Tests pass	no
Fixed tickets
License	MIT

[gregurco]

Hello @macr1408

Thank you for your contribution, but we are supporting Symfony from version 2.7 and I think it's impossible to apply your changes, because event in Symfony 4.2 the first argument is expected to be string, but second to be Event.

See more here: https://github.com/symfony/symfony/blob/4.2/src/Symfony/Component/EventDispatcher/EventDispatcher.php#L37

Applying your changes we will drop the support of Symfony till 4.3.

[macr1408]

@gregurco I totally understand your point, however not merging this will drop support for 4.3 and incoming 4.4 (and the next future versions). I suggest creating a new tag and including this change. It's okay if you want to keep backwards compatibility, specially when there are lots of supported versions, but that makes this bundle outdated.

So, what about two versions of this bundle and everybody is happy? One version for Symfony <= 4.2 and the other one for >= 4.3, so everyone could choose their version from composer

[gregurco]

@macr1408 as I understand, current version of bundle can easy work with 4.x version. Because symfony follows BC rules. Yes, users will have have notice, but it will work. It's more related to 5.x version and of course we can prepare new release for 5.x and drop some too old versions of Symfony.

Am I right? Do you think it's required to be applied today?

[macr1408]

@gregurco Yes, it works, but symfony still logs a message every single time a http request is made, this increases the log file size. In short time the log will be so big that it will occupy the entire server's disk. I don't think disabling deprecations warnings are a good idea neither.

Whether you merge it or not is up to you, I'm just proposing a solution that will make everybody happy.

For me, it's something necessary, I develop code and share it for others to deploy it, right now I need to tell them: "after installing all dependencies, go inside your vendor folder and modify this guzzle file", it destroys the main purpose of using composer.

[gregurco]

@macr1408 I understand. It's very important change and we can't apply it without discussion. I will spend some time to investigate it. Btw, I think these logs are written only in dev mode and doesn't affect prod. So, for me I don't see any reasons to modify vendor.

@florianpreusner what do you think?

[macr1408]

@gregurco Actually I get these messages in production and in development too. I think I have the default symfony config so it's the default expected behaviour. I might be wrong though. Thank you for considering this.

[florianpreusner]

Idea: Creating a branch based on current master and name it symfony_2.7-4.1 (or something similar). Afterwards master can focus on symfony 4.2. First release on this branch will have a new major version.

Disadvantage:

• Keeping both branches "up to date".
• Maybe bundle will not be used that much in Symfony >= 4.3 because of HttpClient

[johnsnowderomania]

My kibana is full of errors due to this. When do you think we can do something with it ?

[gregurco]

@johnsnowderomania why are you logging INFO logs in production? You should log messages from debug to error in case of error and not all the time.

[johnsnowderomania]

@gregurco I'm doing what are you saying but I'm processing multiples messages via queues and I have some errors. I know, I should fix my errors but until then for each error I have I receive this magical info. Hope they will not make this method call obsolete in the near future.

[gregurco]

BC fix will be provided in #265 . Close the PR.