Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
[DependencyInjection] Document FQCN aliases#7656
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| Acme\TransformerInterface: | ||
| alias:rot13_transformer | ||
| public:false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why bother withpublic: false? Let's show the shortAcme\TransformerInterface: '@ rot13_transformer' instead :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
because we don't need this alias in the dumped container, it's better to show the best practices imo ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think we should bother with this kind of optimization that can lead to confusion here. And as it's not specific to autowiring, I should rather be in a dedicated section rather. The notion of public/private is a bit special in Symfony, I wouldn't mix it with autowiring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ok, let's remove it for now, we'll see if others think differently.
theofidry commentedMar 19, 2017
👍 just the |
| autowiring_types:Acme\TransformerInterface | ||
| class:Acme\Rot13Transformer | ||
| Acme\TransformerInterface:@rot13_transformer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The@rot13_transformer string needs to be quoted.
| <serviceid="rot13_transformer"class="Acme\Rot13Transformer"> | ||
| <autowiring-type>Acme\TransformerInterface</autowiring-type> | ||
| </service> | ||
| <serviceid="rot13_transformer"class="Acme\Rot13Transformer" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
for consistency, we should add a blank line after this too
| Fortunately, the FQCN alias is here to specify which implementation | ||
| to use by default. | ||
| ..versionadded::3.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
3.3
xabbuh left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
HeahDude left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
99ba301 toa544200Comparejaviereguiluz commentedApr 28, 2017
@GuilhemN could you please rebase this so we can merge it? Thanks! |
GuilhemN commentedApr 28, 2017
rebased ☺ |
javiereguiluz commentedApr 28, 2017
@GuilhemN thanks for rebasing. This is now merged! |
This PR was merged into the master branch.Discussion----------[DependencyInjection] Document FQCN aliasesFix#7445> Not only do we need to remove autowiring-types, we should show clearly how you can use aliases to choose what class should be autowired for a specific interface/class.>> Also, Stof noted that you can/should mark these aliases as private: we don't need them to be available on the final, cached container.Commits-------a530019 [DependencyInjection] Document FQCN aliases
Fix#7445