Skip to content

Conversation

@xabbuh
Copy link
Member

@xabbuh xabbuh commented Aug 3, 2020

Q A
Branch? master
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

$container->getDefinition('mailer.transports')->setArgument(0, $transports);
$container->getDefinition('mailer.default_transport')->setArgument(0, current($transports));

$container->removeDefinition('mailer.logger_message_listener');
Copy link
Member

Choose a reason for hiding this comment

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

why removing a definition and adding a deprecated alias following it ? Why not creating the right deprecated alias from the beginning ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The mailer.message_logger_listener service is not always defined (only when the profiler is enabled).

Copy link
Member

Choose a reason for hiding this comment

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

but here, you still remove the mailer.logger_message_listener definition all the time to replace it by an alias. So I don't see how this matters.

@fabpot
Copy link
Member

fabpot commented Aug 11, 2020

Thank you @xabbuh.

@fabpot fabpot merged commit 8449f70 into symfony:master Aug 11, 2020
@xabbuh xabbuh deleted the pr-37712 branch August 11, 2020 16:42
fabpot added a commit that referenced this pull request Aug 21, 2020
…logger listener (xabbuh)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[FrameworkBundle] properly choose the best mailer message logger listener

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

Properly merge the changes from #37729 and #37728.

Commits
-------

6cffc79 properly choose the best mailer message logger listener
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
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.

6 participants