[Bundles] Promote the new bundle structure everywhere #19793
Conversation
carsonbot
changed the title
|
Yes, it's perfect timing since we introduced I'll check the documentation later; thanks for the ping! |
carsonbot
added
Status: Needs Work
Status: Needs Review
and removed
Status: Reviewed
Status: Needs Work
labels
|
Thanks for the reviews. I updated the things you asked me to. |
bundles/extension.rst
Outdated
|
|
||
| class AcmeHelloBundle extends AbstractBundle | ||
| { | ||
| public function loadExtension(array $config, ContainerConfigurator $containerConfigurator, ContainerBuilder $containerBuilder): void |
There was a problem hiding this comment.
Just caught this one too (match AbstractBundle::loadExtension() arg names)
public function loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder): void
Linter appears to be complaining about this. Pretty sure this is a false positive in this case.
There was a problem hiding this comment.
We added a rule to explicitly name them like this, not sure its a good idea anymore 🤔
@alamirault were you involved back then with this rule?
There was a problem hiding this comment.
I haven't looked at the code for the rule - but I'm guessing it could be improved by adding a conditional to check if the variable name matches the first|second|third... "word part" of the type name
There was a problem hiding this comment.
We've already reverted most of the changes to the longer name: #18299
Let's remove the rule imho
|
Please rebase to fix DOCtor-RST build, thanks |
After talking with @jrushlow, it's clear that there are some docs that still promote too much the legacy bundle structure, which is inconsistent with the best-practices explained in https://symfony.com/doc/current/bundles.html and https://symfony.com/doc/current/bundles/best_practices.html
So, I propose to update those articles (https://symfony.com/doc/current/bundles/extension.html#loading-services-directly-in-your-bundle-class and https://symfony.com/doc/current/bundles/configuration.html#using-the-abstractbundle-class) to always explain first the simpler solution based on
AbstractBundle.Pinging @yceruto because he's the most active folk on everything related to simplifying bundles. Thanks!