search

plone / plone.app.theming

Integrates the Diazo theming engine with Plone
https://docs.plone.org/external/plone.app.theming/docs/index.html
Other
21 stars 30 forks source link

Invalid dependency on p.a.caching #187

Closed jensens closed 4 years ago

jensens commented 4 years ago

plone.app.caching is considered as a core-addon and nothing in core must depend on it. Only packgae Plone depends on plone.app.caching (same like plone.app.upgrade or plone.api).

This breaks some of my buildouts that are not dependending on Plone, but on Products.CMFPlone only.

@mauritsvanrees Please do not release Plone 5.2.2 with this problem.

cc @MrTango You introduced this import.

I would consider a quick fix and copy over the 4 lines of code at first. Then we could think about moving it to another place as if it is of general interest.

https://github.com/plone/plone.app.theming/blob/f5fc1a3e0be95b921e06c1c0adeb62463c778518/src/plone/app/theming/browser/custom_css.py#L6

https://github.com/plone/plone.app.caching/blob/fba8f21f5c9fc7965f9a99c43dfe6551de1281d5/plone/app/caching/operations/utils.py#L483-L495

mauritsvanrees commented 4 years ago

Good point. Copying it over for now seems best. Later maybe move it to CMFPlone/utils.py?

jensens commented 4 years ago

OK, I prepared a PR.

ale-rt commented 4 years ago

This line: https://github.com/plone/plone.app.theming/pull/188/files#diff-d772ed675c1491b252c7b3e530ee397cR29 needs an import time at the top

jensens commented 4 years ago

This is true, no idea why it worked here and tests were green...

ale-rt commented 4 years ago

I fixed it in #189, the tests are passing because the custom css is not tested see #190.