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

[FrameworkBundle] Simplify "invokable" controllers as services#11193

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

Closed
kbond wants to merge2 commits intosymfony:2.3fromkbond:invokable-controllers
Closed

[FrameworkBundle] Simplify "invokable" controllers as services#11193

kbond wants to merge2 commits intosymfony:2.3fromkbond:invokable-controllers

Conversation

@kbond
Copy link
Member

QA
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PRsymfony/symfony-docs#3966

Simplifies the configuration when defining controllers as services that implement__invoke.

Old:

my_route:path:/pathdefaults:{ _controller: my.controller.service:__invoke }

New:

my_route:path:/pathdefaults:{ _controller: my.controller.service }

@Tobion
Copy link
Contributor

Cannot be merged into 2.3

@kbond
Copy link
MemberAuthor

Ok, I wondered about that - I guess it is a new feature. Do I need to re-submit?

Copy link
Member

Choose a reason for hiding this comment

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

please useelseif instead of adding another if inside theelse.

@stof
Copy link
Member

This should indeed go into master.

@kbond
Copy link
MemberAuthor

Ok, is it difficult for a merger to merge this into master or should I open a new PR?

@stof
Copy link
Member

I think it can be moved automatically without conflict. This can has not changed in later releases IIRC

@stof
Copy link
Member

👍

@Tobion
Copy link
Contributor

Please add a test for this, then I'm 👍 for merge into master

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably previous condition also must check whether container has such service.

Copy link
Member

Choose a reason for hiding this comment

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

nope. The previous condition is explicitly a service syntax. Not checking if the container has a service means you get an exception telling you the service does not exist in case you made a typo.

@kbond
Copy link
MemberAuthor

OK. Will add.

It looks like there are no tests for this class. Or am I missing something?

@kbond
Copy link
MemberAuthor

I added tests forSymfony/Bundle/FrameworkBundle/Controller/ControllerResolver - should cover this and all existing scenarios.

Copy link
Contributor

Choose a reason for hiding this comment

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

authors are not added to tests

@romainneutron
Copy link
Contributor

👍 for a merge in master

@fabpot
Copy link
Member

Thank you@kbond.

weaverryan added a commit to symfony/symfony-docs that referenced this pull requestAug 16, 2014
…ler services (kbond)This PR was merged into the master branch.Discussion----------[Cookbook][Controller] Add note about invokable controller services| Q             | A| ------------- | ---| Doc fix?      | no| New docs?     | yes (symfony/symfony#11193)| Applies to    | 2.6+| Fixed tickets | -Commits-------9879ee6 Add note about invokable controller services
fabpot added a commit to sensiolabs/SensioFrameworkExtraBundle that referenced this pull requestNov 10, 2014
…verterListener (jameshalsall)This PR was squashed before being merged into the 3.0.x-dev branch (closes#333).Discussion----------Adding support for invokable controllers to the ParamConverterListenerInvokable controller support was added to Symfony insymfony/symfony#11193.The problem was that the `ParamConverterListener` did not support passing an object as the controller, this PR adds that support.Commits-------1b83587 Adding support for invokable controllers to the ParamConverterListener
fabpot added a commit that referenced this pull requestNov 12, 2014
… in the RequestDataCollector (jameshalsall)This PR was submitted for the master branch but it was merged into the 2.5 branch instead (closes#12443).Discussion----------[HttpKernel][2.6] Adding support for invokable controllers in the RequestDataCollector| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | N/A| License       | MIT| Doc PR        | N/AAs part of#11193 support for controllers using `__invoke()` was added.The `RequestDataCollector` did not support controllers that were defined in the routing as...```phproute_name:    path: /{id}    defaults: { _controller: acme_app.page.controller.page }    requirements:        id: \d+```Where the controller was defined as...```phpclass PageController{    public function __invoke()    {        //    }}```This PR adds that support. Tests have been updated.Commits-------f1d043a [HttpKernel][2.6] Adding support for invokable controllers in the RequestDataCollector
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kbond@Tobion@stof@romainneutron@fabpot@megazoll

[8]ページ先頭

©2009-2025 Movatter.jp