search

sulu / SuluThemeBundle

MIT License
18 stars 15 forks source link

Symfony 5 compatibility and Switch to SyliusThemeBundle #17

Closed alexander-schranz closed 3 years ago

alexander-schranz commented 4 years ago
Q A
Bug? no
New Feature? yes
Bundle Version for 2.1
Sulu Version 2.1
Browser Version -

Actual Behavior

Currently its not possible to use the SuluThemeBundle with Symfony 5 because its build on top of the LiipThemeBundle.

Expected Behavior

After I created an issue on the LiipThemeBundle and talked to @lsmith77 he did point me to an alternative from https://github.com/Sylius/SyliusThemeBundle. After having a quick look at it I think it would be a great alternative to the current LiipThemeBundle.

Possible Solutions

How to upgrade

The upgrade for exists projects looks for me simple as it seems you just need to create inside the theme folder a .json file. https://github.com/Sylius/SyliusThemeBundle/blob/master/docs/your_first_theme.md with the theme configuration and it will then read the files from there. Things like parent themes are a lot easier in the SyliusThemeBundle configurable as in the LiipThemeBundle.

/cc @sulu/core-team What do you think?

danrot commented 4 years ago

I am generally fine with the SyliusThemeBundle, especially since it seem to be much more active. But I am wondering if we can make this change without any BC breaks. I mean we probably can, but it is questionable to me if it is worth the effort and risk (there'll probably be differences between these two libraries requiring us to write some translation code or we overlook them).

I can think of two strategies to make this easier:

  1. Release a new version 3.0, so we are allowed to make BC breaks, which would make the switch from the LiipThemeBundle to the SyliusThemeBundle a lot easier for us, and people would be aware that stuff might go south.
  2. Integrate this bundle into the core repository, which would make maintenance for us a bit easier, but future upgrades might take even longer, because there is one more dependency to take care of when e.g. a Symfony major update is released. However, it would also make sense, because some of the theme handling is already included in the core repository. Could also be done by adding it as an optional dependency.
alexander-schranz commented 4 years ago

@danrot I basically have no strong opinion on this if we make 2.1 or 3.0. I see there not so much problems. Basically I have no problem to make this bundle part of the core as long as the dependencies to the SyliusThemeBundle is optional, but at current state I would prefer keeping the code here so we can release a Symfony 5 Compatible theme version as soon as possible.