Skip to content
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

Symfony 5 compatibility and Switch to SyliusThemeBundle #17

Closed
alexander-schranz opened this issue Jun 30, 2020 · 2 comments · Fixed by #19
Closed

Symfony 5 compatibility and Switch to SyliusThemeBundle #17

alexander-schranz opened this issue Jun 30, 2020 · 2 comments · Fixed by #19

Comments

@alexander-schranz
Copy link
Member

alexander-schranz commented Jun 30, 2020

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

  • Make LiipThemeBundle a optional dependency
  • ~~If LiipThemeBundle is installed set correct theme on request
  • Make SyliusThemeBundle a optional dependency
  • If SyliusThemeBundle is installed set correct theme on request (see docs)
  • Remove LiipThemeBundle integration
  • Add SyliusThemeBundle integration

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?

@alexander-schranz alexander-schranz changed the title Symfony 5 compatibility Symfony 5 compatibility and Switch to SyliusThemeBundle Jun 30, 2020
@danrot
Copy link
Contributor

danrot commented Jul 16, 2020

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
Copy link
Member Author

@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.

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

Successfully merging a pull request may close this issue.

2 participants