avanzu / AdminThemeBundle

Admin Theme based on the AdminLTE Template for easy integration into symfony
MIT License
281 stars 149 forks source link

Load commands explicit as auto-dicovery way deprecated in Symfony 3.4 #215

Closed kevinpapst closed 5 years ago

kevinpapst commented 6 years ago

Hi guys,

with Symfony 3.4 auto-discovery for Commands was deprecated: https://github.com/symfony/symfony/issues/23488 https://github.com/symfony/symfony/pull/23805

That means the AdminTheme Commands avanzu:admin:* are not available in case of deactivated auto-wiring. With Symfony 4 this is the default behaviour and the commands are never available.

While migrating a Project of mine to SF4, I couldn't properly initialize the theme as the commands wheren't available and I had to add the attached changes to services.yml to make them callable.

These 3 lines register the command directory for Service discovery and the commands will be available when executing bin/console.I tested it only with Symfony 4, but according to Symfony core devs it will at least work with 3.4 as well: https://github.com/symfony/symfony/issues/23488#issuecomment-315598669

shakaran commented 6 years ago

@kevinpapst thanks for this, but for merge in master, I need really know if it will not break symfony 2.X and 3.X version, so we need ensure the behaviour for current supported SF versions

kevinpapst commented 6 years ago

I totally understand that. Is there a demo app repo which we can use for testing? If not, we should add one, probably forking https://github.com/symfony/demo/ and adding AdminThemeBundle on top, that should make it easy to test things out. I do not have any experience with Symfony 2, but I can help with testing Symfony 3.3 and lower.

Do you plan to support Symfony 2 and 3 for the v2 release? And what is your plan for deprecating/skipping support old Symfony versions? I am curious, because I have some more adjustments for Symfony 4 which I could sent in, but to be honest: I don't want to test the old versions for every change.

EDIT: There is a fork for the Symfony 3.4 and 4 users which includes these changes: https://github.com/kevinpapst/AdminLTEBundle

shakaran commented 5 years ago

All this changes has been merged now via https://github.com/avanzu/AdminThemeBundle/commit/e449c13d956109bf3c76f2e53c58341f9516500e

In the new branch "experimental-symfony-4.x"

https://github.com/shakaran/AdminThemeBundle/tree/experimental-symfony-4.x

Since I am having some problems for update the packagist.org package (I already requested to be maintainer to avanzu main owner, but it could take time to it), I am maintaining the same commits and duplicating the work in a personal repo and packagist account:

composer require shakaran/admin-theme-bundle:dev-experimental-symfony-4.x

https://packagist.org/packages/shakaran/admin-theme-bundle