Skip to content

[HttpKernel] Add convenient method ArgumentResolver:: getDefaultArgumentValueResolvers #19011

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

Merged

Conversation

romainneutron
Copy link
Contributor

@romainneutron romainneutron commented Jun 9, 2016

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

I realized while implementing my own ArgumentValueResolver that registering a new one is not easy, you have to give the whole list of resolver instead of pushing your one in the stack.

@stof
Copy link
Member

stof commented Jun 9, 2016

IIRC, making the service immutable was done on purpose

@stof
Copy link
Member

stof commented Jun 9, 2016

it make make sense to have a static method returning the array of builtin value resolvers though, to make it easier to register them when adding additional ones

@romainneutron romainneutron force-pushed the convenient-addArgumentValueResolver-3.2 branch from 6d6018e to 6fdfc92 Compare June 9, 2016 15:03
@romainneutron
Copy link
Contributor Author

I've updated the PR according to @stof comment

@romainneutron romainneutron changed the title [HttpKernel] Add convenient method ArgumentResolver::addArgumentValueResolver [HttpKernel] Add convenient method ArgumentResolver:: getDefaultArgumentValueResolvers Jun 9, 2016
@linaori
Copy link
Contributor

linaori commented Jun 9, 2016

The static method is nice, adds a good DX when used outside of the full framework.

@fabpot
Copy link
Member

fabpot commented Jun 10, 2016

Thank you @romainneutron.

@fabpot fabpot merged commit 6fdfc92 into symfony:master Jun 10, 2016
fabpot added a commit that referenced this pull request Jun 10, 2016
…getDefaultArgumentValueResolvers (romainneutron)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[HttpKernel] Add convenient method ArgumentResolver:: getDefaultArgumentValueResolvers

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

I realized while implementing my own `ArgumentValueResolver` that registering a new one is not easy, you have to give the whole list of resolver instead of pushing your one in the stack.

Commits
-------

6fdfc92 [HttpKernel] Add convenient method ArgumentResolver::addArgumentValueResolver
@romainneutron romainneutron deleted the convenient-addArgumentValueResolver-3.2 branch June 10, 2016 13:32
@fabpot fabpot mentioned this pull request Oct 27, 2016
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.

4 participants