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

[Hackday][Messenger] Add an alias for transport.symfony_serializer so SerializerInterface can be autowired #29517

Merged

Conversation

Projects
None yet
6 participants
@karser
Copy link
Contributor

commented Dec 8, 2018

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

cc @thePanz

Use case:
Before:

    Pnz\Messenger\FilesystemTransport\FilesystemTransportFactory:
        arguments:
            - '@messenger.transport.symfony_serializer'
        tags: ['messenger.transport_factory']

After:

    Pnz\Messenger\FilesystemTransport\FilesystemTransportFactory:
        tags: ['messenger.transport_factory']
@stof

This comment has been minimized.

Copy link
Member

commented Dec 8, 2018

#SymfonyConHackday2018

@chalasr

chalasr approved these changes Dec 8, 2018

Copy link
Member

left a comment

👍 with minor comment.
Sounds more like a feature to me, better merge it on master I'd say.

@ogizanagi

This comment has been minimized.

Copy link
Member

commented Dec 8, 2018

I wondered about this alias before, but the serializer is configurable through the framework.messenger.serializer.id config.
So what would you expect to be injected when setting this option to a different value than the symfony_serializer and typehinting Messenger's SerializerInterface?
Shouldn't it alias messenger.transport.serializer instead?

@chalasr

This comment has been minimized.

Copy link
Member

commented Dec 8, 2018

Status: needs work
(#29517 (comment))

@chalasr

chalasr approved these changes Dec 9, 2018

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Dec 9, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Dec 9, 2018

I think we prefer defining in xml and removing in the extension: that makes the xml more useful to eg IDE plugins that want to use it.

@karser

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2018

@nicolas-grekas Did you mean this? please review

@nicolas-grekas nicolas-grekas modified the milestones: 4.2, next Dec 13, 2018

@nicolas-grekas nicolas-grekas changed the base branch from 4.2 to master Dec 13, 2018

@nicolas-grekas nicolas-grekas force-pushed the karser:add-alias-to-messenger-serializer branch from 218ba84 to 2f0e948 Dec 13, 2018

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Dec 13, 2018

Thank you @karser.

@nicolas-grekas nicolas-grekas merged commit 2f0e948 into symfony:master Dec 13, 2018

1 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
fabbot.io Your code looks good.
Details

nicolas-grekas added a commit that referenced this pull request Dec 13, 2018

feature #29517 [Hackday][Messenger] Add an alias for transport.symfon…
…y_serializer so SerializerInterface can be autowired (karser)

This PR was submitted for the 4.2 branch but it was squashed and merged into the 4.3-dev branch instead (closes #29517).

Discussion
----------

[Hackday][Messenger] Add an alias for transport.symfony_serializer so SerializerInterface can be autowired

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| License       | MIT

cc @thePanz

Use case:
Before:
```
    Pnz\Messenger\FilesystemTransport\FilesystemTransportFactory:
        arguments:
            - '@messenger.transport.symfony_serializer'
        tags: ['messenger.transport_factory']
```

After:
```
    Pnz\Messenger\FilesystemTransport\FilesystemTransportFactory:
        tags: ['messenger.transport_factory']
```

Commits
-------

2f0e948 [Hackday][Messenger] Add an alias for transport.symfony_serializer so SerializerInterface can be autowired

@nicolas-grekas nicolas-grekas removed this from the next milestone Apr 30, 2019

@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Apr 30, 2019

@fabpot fabpot referenced this pull request May 9, 2019

Merged

Release v4.3.0-BETA1 #31435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.