[DI] Documenting Abstract Bundle and Extension #16801
Conversation
|
About CI jobs, Is there any way to know which code block is failing? and how these failures can be fixed?: |
…le` is used (yceruto) This PR was merged into the 6.1 branch. Discussion ---------- [HttpKernel] New bundle path convention when `AbstractBundle` is used | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | symfony/symfony-docs#16801 Addressing this comment symfony/symfony-docs#16801 (comment) Since Symfony 6.1 when any bundle starts using the new `AbstractBundle` and the bundle is placed in the `src/` directory we automatically update this path to the parent directory, so we don't have to do it manually to use [the modern bundle directory structure](https://symfony.com/doc/current/bundles/best_practices.html#directory-structure). As far as I can see, there is no BC break on this proposal as this is a new class introduced in 6.1 and you have to update your code to make this happen. I will add a note about this subject in the doc too. Commits ------- 1193ed2 New bundle path convention when AbstractBundle is used
75d9b9b to
d0cba72
Compare
There was a problem hiding this comment.
Hi @yceruto! Thanks again for all these amazing docs.
I've pushed a commit to your branch doing a bit more radical rewrite. Please have a look at the changes. This PR is ready to merge if you approve them imho :)
As I said on Slack, we might want to rewrite all bundle docs in the future, to swap the order (first AbstractBundle, then Extension+Configuration). Yet, the current changes are perfect for the 6.1 release already I think.
| @@ -205,6 +212,39 @@ Before continuing, run this command to add support for the new dependencies: | |||
|
|
|||
| $ composer require symfony/yaml symfony/twig-bundle symfony/web-profiler-bundle doctrine/annotations | |||
|
|
|||
| Next, create a new extension class that defines your app configuration and | |||
There was a problem hiding this comment.
To be honest, I'm not really sure if this is a good place to document AbstractExtension. What is its use-case when using MicroKernelTrait?
If we want to keep it, I would suggest moving the changes in this document to a new section below this one. Currently, I'm afraid the doc can make people think that they must use the app extension (or build() method) when using the micro kernel.
There was a problem hiding this comment.
To be honest, I'm not really sure if this is a good place to document AbstractExtension. What is its use-case when using MicroKernelTrait?
Apps on bundle-less approach still need to define custom configuration, and it's only possible through a new extension, even if the kernel class can implement the ExtensionInterface + Configuration it's better now by creating a new extension class and extending from AbstractExtension.
If we want to keep it, I would suggest moving the changes in this document to a new section below this one. Currently, I'm afraid the doc can make people think that they must use the app extension (or build() method) when using the micro kernel.
I think you're right, it could be better in a new section with the proper explanation.
|
@wouterj thanks a lot for your improvements, it looks better now 🙏 |
|
This was finally merged 🎉 Yonel, thanks again for such a nice contribution! Thanks to reviewers too! |
Fixes #16669
PR symfony/symfony#43701
ping @Nyholm @wouterj as you were interested in this feature/doc. Any suggestion to improve the wording is more than welcome!
I also updated other places to be aligned with the best practices doc.
I didn't remove the current convention to create bundles/extensions/configurations because they are still valid until 6.4 becomes the last LTS available. However, it's ok for apps using AbstractExtension.