EZP-30627: Unified Twig helpers naming #2663
Conversation
pawbuj
requested review from
adamwojs,
Nattfarinn,
alongosz,
mikadamczyk,
ViniTou and
webhdx
|
Following up the discussion at #2633 : were the current names of twig helpers already declared as deprecated? If not, I'd politely suggest to keep around the old name (and add a deprecation notice for any usage) for at least the duration of one major kernel version. Twig helpers tend to be one of the apis most used by every ez developer... |
alongosz
changed the title
| * | ||
| * @return string|null | ||
| */ | ||
| public function getTranslatedProperty(ValueObject $object, $property, $forcedLanguage = null) |
There was a problem hiding this comment.
Why did you drop it? Seems out of scope.
There was a problem hiding this comment.
because I dropped ez_trans_prop
There was a problem hiding this comment.
because I dropped
ez_trans_prop
Not following, scope was to unify naming, not drop features. Is there a replacement for this one?
| @@ -39,6 +39,6 @@ public function getName() | |||
| */ | |||
| public function getGlobals() | |||
| { | |||
| return array('ezpublish' => $this->globalHelper); | |||
| return ['ez_app' => $this->globalHelper]; | |||
There was a problem hiding this comment.
Rather ezplatform, it's about the Product.
There was a problem hiding this comment.
Symfony provides app global helper , ezPlatform provides ez_app . So, it is because of naming compatibility with symfony.
There was a problem hiding this comment.
Still -1 from my side.
// Edit as to why:
We have product, not project. Moreover if we wanted to distinguish configuration of eZ Platform and e.g. eZ Commerce, we couldn't follow any convention with such naming.
On top of that, it's also about Product placement.
There was a problem hiding this comment.
In most cases (with exceptions of course, but this is not the case here) renaming ezpublish to ezplatform is recommended.
eZ/Publish/Core/MVC/Symfony/Templating/Twig/Extension/RichTextExtension.php
Show resolved
Hide resolved
alongosz
changed the title
|
Due to large amount of changes for Symfony 4 and cleanup around templates directory structures, this change does not add any significant work which needs to be done during upgrade anyway, especially that we are considering to provide a simple bash/PHP command or entire compatibly bundle. Subject is still open. |
|
@pawbuj besides my previous review please list those changes in |
|
@lserwatka does it mean that the next version of eZ Platform is going to be 100% Symfony 4/5 compliant? |
masterrelated to : ezsystems/ezplatform-admin-ui#1008
TODO:
$ composer fix-cs).