Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@karser
Copy link
Contributor

@karserkarser commentedDec 8, 2018•
edited by nicolas-grekas
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT

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
Copy link
Member

stof commentedDec 8, 2018

#SymfonyConHackday2018

Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

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

@ogizanagi
Copy link
Contributor

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

chalasr, karser, and jvasseur reacted with thumbs up emoji

@chalasr
Copy link
Member

Status: needs work
(#29517 (comment))

@nicolas-grekasnicolas-grekas added this to the4.2 milestoneDec 9, 2018
@nicolas-grekas
Copy link
Member

nicolas-grekas commentedDec 9, 2018•
edited
Loading

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
Copy link
ContributorAuthor

@nicolas-grekas Did you mean this? please review

@nicolas-grekasnicolas-grekas modified the milestones:4.2,nextDec 13, 2018
@nicolas-grekasnicolas-grekas changed the base branch from4.2 tomasterDecember 13, 2018 10:40
@nicolas-grekasnicolas-grekasforce-pushed theadd-alias-to-messenger-serializer branch from218ba84 to2f0e948CompareDecember 13, 2018 10:40
@nicolas-grekas
Copy link
Member

Thank you@karser.

karser reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas merged commit2f0e948 intosymfony:masterDec 13, 2018
nicolas-grekas added a commit that referenced this pull requestDec 13, 2018
…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       | MITcc@thePanzUse 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-grekasnicolas-grekas removed this from thenext milestoneApr 30, 2019
@nicolas-grekasnicolas-grekas added this to the4.3 milestoneApr 30, 2019
@fabpotfabpot mentioned this pull requestMay 9, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@chalasrchalasrchalasr approved these changes

+1 more reviewer

@ogizanagiogizanagiogizanagi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

6 participants

@karser@stof@ogizanagi@chalasr@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp