Skip to content

[Kernel] [Bundle] Setting up bundle dependencies #13505

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

Closed
Pierstoval opened this issue Jan 24, 2015 · 13 comments
Closed

[Kernel] [Bundle] Setting up bundle dependencies #13505

Pierstoval opened this issue Jan 24, 2015 · 13 comments

Comments

@Pierstoval
Copy link
Contributor

In some old topics from the Standard-Edition:
#639 - Improving the Symfony Standard Edition
And especially this one:
#608 - Bundle dependencies on other bundles as defined by the bundle instead of AppKernel

I wanted to open a new issue to discuss about the bundle dependency feature. In fact, I made a little test on my local fork with some "test" bundles, and it seems to work correctly, but in fact, I'm not a complete expert of Symfony core system, so I wanted to first discuss about this feature that @fabpot seemed not to find a solution for (that's why #608 has been closed) , and propose in the same way the pull-request with some little modifications inside the Kernel and Bundle classes.

The PR will be referenced just after this post, so please, what do you think ?

@linaori
Copy link
Contributor

linaori commented Jan 24, 2015

I'm very glad to see this is being given more attention again 👍

@Pierstoval
Copy link
Contributor Author

The best would be to have Symfony Standard edition using this, so there will only be only one bundle registered at first in AppKernel : Symfony\StandardEditionBundle() dreaming ☁️

@jakzal
Copy link
Contributor

jakzal commented Jan 25, 2015

@Pierstoval I don't see any value in StandardEditionBundle. It would make things less clear, as you suddenly can't see what bundles are registered. You're not adding them to the kernel yourself as they're there when you initialize the project, so I really don't see an issue here. It would also complicate removal of bundles. For example, if I wanted to remove doctrine from a project I would need to replace StandardEditionBundle with all the bundles that it registers and then remove DoctrineBundle.

@Pierstoval
Copy link
Contributor Author

@jakzal You're right, but I'm thinking about beginners that don't yet know the concept of bundle. If the doc says that a bundle has to be registered in AppKernel, maybe he'll wonder what are the actually registered bundles. If he only sees Symfony\StandardEditionBundle , it's more verbose to him than Symfony\Bundle\MonologBundle\MonologBundle which is not verbose for beginners (monolog, even if it's terminated with "log", is not explicitly described as a logger bundle), added to the fact that beginners won't use the logger until they're familiar with either logging or debugging in Symfony.

Plus, the most important to me is to simplify the integration of a "Bundled bundle", as in #695 (in symfony-standard). The example of SonataPageBundle, which depends on more than 10 bundles ( ❗ ) would be simpler to register if you only have to add SonataPageBundle in your kernel.
In fact, when you install a new bundle, you want to use this bundle, and if it depends on other bundles (like SonataCacheBundle or SonataCoreBundle in our case) , you certainly won't mind how they work : you'll focus on the feature you want (a "page system" in our case).

For the case in which you'd want to remove DoctrineORM in your project, you're right. Probably the best solution for this is to use a powerful IDE that will allow you to Ctrl+Click on the class name to check all the bundle dependencies directly in the class, and then add them manually.
Anyway, the example of StandardEditionBundle might be a bit bad, and in fact, I wouldn't add "all" bundles to it, only some "almost mandatory" ones, like Security, Twig, FrameworkBundle, Monolog and SensioFrameworkExtraBundle. By the way, it can only come with a consensus, and it's not the principal topic ;)

@cjunge-work
Copy link

+1. I think this is a great idea. If someone needs to disable a particular dependant bundle, then it can be controlled via config (eg. for the Doctrine case) or looking in the extension/configuration and copy & pasting.

@stof
Copy link
Member

stof commented Jan 30, 2015

If someone needs to disable a particular dependant bundle, then it can be controlled via config (eg. for the Doctrine case) or looking in the extension/configuration and copy & pasting.

@cjunge-work the config is about configuring registered bundles. Enabling bundles based on the config creates a circular loop (you need to register bundles before being able to provide config for them)

@Pierstoval
Copy link
Contributor Author

@stof maybe @cjunge-work was talking about something like the old #598 Put enabled bundles in a configuration file in symfony standard edition :)

@cjunge-work
Copy link

@stof: I meant supporting a specific way to turning off bundles, in the config file. I know that the Bundle is instantiated, but the functionality can be disabled via config (eg. a lot of bundles don't work correctly until their config is enabled, sometimes as simple as bundle-name: ~).

Otherwise, the suggestion of @Pierstoval would suite.

Just throwing it out there... sure would help with those packages that require multiple bundles. As an example, I've started using SonataAdminBundle and SonataUserBundle in a project... the amount of back & forth over enabling bundles, setting config, etc is confusing!

@andrerom
Copy link
Contributor

Two PR's now exists for this issue, direction from core devs needed to help move this forward:

#13506
#13945

PS: None of them uses config files, so no request time overhead here besides additional function calls, which can be further optimized later (3.0)? for prod by introducing faster boot process, by means of caching both the existing parent/child and dependencies calculations, or even more..

@lsmith77
Copy link
Contributor

@symfony/deciders I would like us to discuss this if we are going to put it into 2.7 .. obviously now we can also decide to delay it until 2.8 LTS but the decision should be conscious rather than accidental.

@fabpot
Copy link
Member

fabpot commented Apr 16, 2015

2.7 beta1 is out, so no new features can be added to it. It would be for 2.8.

@Pierstoval
Copy link
Contributor Author

Let's update #13945 in favor of the 2.8 branch then.
Ping @andrerom 😄

@andrerom
Copy link
Contributor

Ok, I will do that tomorrow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants