Skip to content

[Messenger] Remove the default transport if no serializer service #26649

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

Conversation

sroze
Copy link
Contributor

@sroze sroze commented Mar 23, 2018

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

If the serializer service does not exist, we remove the default transport services as they rely on it.

@linaori
Copy link
Contributor

linaori commented Mar 23, 2018

Might be a silly question, but why add it in the first place? Why not only add it if the serializer isn't available?

@chalasr
Copy link
Member

chalasr commented Mar 23, 2018

@iltar Good question, AFAIK we use that approach in general. I guess having as much services as possible declared in xml is good for discoverability

@sroze sroze merged commit 2bcf93d into symfony:master Mar 26, 2018
sroze added a commit that referenced this pull request Mar 26, 2018
…` service (sroze)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[Messenger] Remove the default transport if no `serializer` service

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ø
| License       | MIT

If the `serializer` service does not exist, we remove the default transport services as they rely on it.

Commits
-------

2bcf93d Remove the default transport if no serializer
@sroze sroze deleted the messenger-remove-default-transport-if-no-serializer branch March 26, 2018 20:00
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