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] Clarify the#[Target] attribute#19789
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
ghost commentedApr 14, 2024 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
Good clarification. I've never understood how this attribute works 😄 I had two interpretations but both wrong. Btw. I think this should also be clarified in the source code above $name constructor parameter doc block and class doc block (the latter is a little better but still can be improved).https://github.com/symfony/symfony/blob/7.1/src/Symfony/Component/DependencyInjection/Attribute/Target.php |
Uh oh!
There was an error while loading.Please reload this page.
210f1a5 to06d88b6CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
OskarStark commentedApr 19, 2024
@nicolas-grekas a final review from your side would be helpful, thanks |
06d88b6 to2fb1adaComparenicolas-grekas commentedApr 22, 2024
LGTM but maybe we should tell about the debug:autowiring command, which does report the target name sincesymfony/symfony#50718? |
HypeMC commentedApr 22, 2024
@nicolas-grekas Good idea, I've created afollowup PR since this one targets 5.4, and the feature was introduced in 6.4. |
javiereguiluz commentedJun 24, 2024
@HypeMC thanks for this contribution and sorry it took us so long to merge it. |
javiereguiluz commentedJun 24, 2024
@HypeMC I upmerged this in 6.4, 7.0, 7.1 and 7.2 branches. I'm not sure I did everything right, because in 6.4 we show both upperTransformer and shoutyTransformer. I don't know if both are correct or the "shouty" thing is a leftover from 5.4 branch and 6.4 branch was wrong before merging this PR. If you can, please review it. Seead1e3b2 |
…2 (HypeMC)This PR was squashed before being merged into the 6.4 branch.Discussion----------[DependencyInjection] Clarify the `#[Target]` attribute 2Follow-up to#19789, adds a [paragraph](bd00116) about the `debug:autowiring` command.Commits-------81cc7f0 [DependencyInjection] Clarify the `#[Target]` attribute 2
HypeMC commentedJun 24, 2024
@javiereguiluz Yep, everything looks ok, I see you've also merged the follow up PR. |
The
#[Target]attribute seems to be a constant source of confusion for developers, as evident by:#[Target]attribute in Symfony 6.3 symfony#50541Also, the example given is either unclear or just wrong. Hopefully, this helps clarify things.