search

symfony-cmf / content-bundle

Provides some basic document classes and controllers for modeling content
https://cmf.symfony.com
19 stars 32 forks source link

move core, menu and routing bundle to dev #178

Closed ElectricMaxxx closed 6 years ago

ElectricMaxxx commented 6 years ago

just for a try.

dbu commented 6 years ago

you would need the install test to see if this works at all. even then we don't know if its usable alone. you would need to look through the code to see where each of those bundles is used.

dbu commented 6 years ago

this is now in conflict with master.

ElectricMaxxx commented 6 years ago

Indeed, maybe we schould go over 2.0 branch, release and make a PR against master then. Or shall i rebase only?

dbu commented 6 years ago

you need to rebase this branch anyways. the 2.0 branch should always be merged to master after a commit has been merged to 2.0, master should not lag behind.

ElectricMaxxx commented 6 years ago

So rebase on master -> merge into 2.0 -> merge 2.0 into master, release 2.0.1 on branch 2.0 (and when we see installation test green on master)

dbu commented 6 years ago

i would not merge this to 2.0, only to master directly. and a PR starting from master can't be merged to 2.0 otherwise we would essentially merge master to 2.0 which would be wrong.

ElectricMaxxx commented 6 years ago

Instellation test shows the dependency on templating only now.

ElectricMaxxx commented 6 years ago

We should require twig-bundle then, cause templating component is kind of deprecated. We should do an equal split as done in routing-bundle.

dbu commented 6 years ago

ah true, twig bundle it should be then. and we should use the twig service instead of the templating component. is that possible without a BC break of the content controller? i guess we can remove the typehint on the constructor and accept both classes (templating or the twig equivalent).

what do you mean by "equal split"?

ElectricMaxxx commented 6 years ago

We changed the argument of the service depending if twig is available: https://github.com/symfony-cmf/routing-bundle/commit/36ea55cb3f8234db16a4cd997703edeae838e947 doing so we do not need to remove a interface, we just need to inject the correct service.

dbu commented 6 years ago

ah, right. yeah lets do that here too.

dbu commented 6 years ago

188 fixed the templating problem (by supporting twig + requiring templating)