Description Enhancer integration #29

dantleech · 2016-05-28T13:48:26Z

Integrates description enhancers (replaces #26 ).

cmf_resource:
    description:
        enhancers: [ "sonata_admin", "foobar" ]

dantleech · 2016-05-28T13:56:27Z

DependencyInjection/Compiler/DescriptionEnhancerPass.php

+        foreach ($inactiveEnhancers as $inactiveEnhancer) {
+            $container->removeDefinition($serviceId);
+            unset($enhancers[$inactiveEnhancer]);
+        }


This shifts the responsiblity of marking the services as private

I would not do this, it'll add unexpected behaviour. (and marking all services private by default is a very good practice)

Btw, the current code doesn't work, $serviceId will always have the same value (the last value of the previous loop) as it isn't used in this foreach loop.

I can imagine people running into problems and we answer them with "you must mark the service as private" . But such cases should be rare, I don't care very much, will change it.

why "must mark private"? is there a problem if a public unused service is lying around? (apart from cluttering the container and the risk that people fetch it from the container to do stuff)

We need to remove the unused enhancers because they may contain references to services which don't exist (e.g. sonata.admin_pool)- and I believe that will cause the container to fail.

wouterj · 2016-05-29T09:49:16Z

The XSD file also has to be updated.

dantleech · 2016-05-29T14:41:21Z

Resources/config/description.xml

+
+    </services>
+</container>
+


dantleech · 2016-06-05T10:18:15Z

The XSD file also has to be updated.

@wouterj I don't think we have one yet.

dantleech · 2016-06-05T10:26:22Z

composer.json

-        "symfony-cmf/resource": "^1.0"
+        "symfony/options-resolver": "^2.7|3.*",
+        "symfony-cmf/resource": "dev-admin_metadata",
+        "puli/repository": "@beta"


I'm not sure we can avoid doing this. The packages minimum-stability is dev, so the implicit Puli version is dev-master, which is "behind" @beta (it has Resource instead of PuliResource). If we remove the minimum-stability then we still need to specify the unstable @beta version.

The problem here is that both 1.1 and 1.0 aren't stable of Puli. Our version constraint in symfony-cmf/resource is something like ^1.0.0-betaN. As dev versions are allowed, puli will use 1.1.0-dev (as it's the highest dev version in the version constraint).

If we would do ~1.0.0-betaN in symfony-cmf/resource, things work as expected (it'll install the 1.0 dev branch and not master).

nope, still doesn't work. installs dev-master, at least with ^, do you think it will work differently with ~ ?

dantleech · 2016-06-05T10:33:43Z

Happy for this to be merged once #29 is done (and the composer.json has been updated here).

wouterj · 2016-06-05T11:47:36Z

Resources/config/description.xml

+            <tag name="cmf_resource.description.enhancer" alias="sonata_admin" />
+        </service>
+
+        <service id="cmf_resource.description.enhancer.sylius_resource" class="Symfony\Cmf\Component\Resource\Description\Enhancer\Sylius\ResourceEnhancer">


let's make all these services private.

dantleech · 2016-06-15T08:12:15Z

Updated

dantleech · 2016-06-20T07:41:59Z

@wouterj good to merge?

wouterj · 2016-06-20T08:25:43Z

Resources/config/description.xml

+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <parameters>
+        <parameter key="cmf_resource.registry.container.class"></parameter>


should be removed

dantleech added the wip/poc label May 28, 2016

dantleech mentioned this pull request May 28, 2016

Support for enhancers #26

Closed

dantleech closed this May 28, 2016

dantleech force-pushed the description_enhancers branch from cc25d2b to 03ba657 Compare May 28, 2016 13:52

dantleech removed the wip/poc label May 28, 2016

dantleech reopened this May 28, 2016

dantleech added the wip/poc label May 28, 2016

dantleech force-pushed the description_enhancers branch 2 times, most recently from d72e4f5 to ea041bf Compare May 28, 2016 13:54

dantleech reviewed May 28, 2016
View reviewed changes

dantleech reviewed May 29, 2016
View reviewed changes

Resources/config/description.xml

</services>

</container>

Copy link

Member Author

dantleech May 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dd

dantleech mentioned this pull request May 31, 2016

Resource debug / describe command. #30

Open

dantleech added 3 commits June 5, 2016 11:14

Description Enhancer integration

604836f

Added options resolver dep.

bf42ff0

Added DescriptionExtension

fc12126

dantleech force-pushed the description_enhancers branch 2 times, most recently from c757e3a to b0a30c5 Compare June 5, 2016 10:16

dantleech force-pushed the description_enhancers branch from b0a30c5 to e118fa7 Compare June 5, 2016 10:18

dantleech reviewed Jun 5, 2016
View reviewed changes

dantleech force-pushed the description_enhancers branch from e118fa7 to 8574363 Compare June 5, 2016 10:32

wouterj reviewed Jun 5, 2016
View reviewed changes

dantleech force-pushed the description_enhancers branch from 8574363 to 85b89fd Compare June 15, 2016 08:10

Twig Extension tests

157423c

dantleech force-pushed the description_enhancers branch from 85b89fd to 157423c Compare June 15, 2016 08:12

wouterj reviewed Jun 20, 2016
View reviewed changes

wouterj merged commit 20eee84 into master Jun 20, 2016

wouterj deleted the description_enhancers branch June 20, 2016 08:26

wouterj removed the wip/poc label Jun 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Description Enhancer integration #29

Description Enhancer integration #29

dantleech commented May 28, 2016

dantleech May 28, 2016

wouterj May 29, 2016

dantleech May 29, 2016

dbu May 31, 2016 via email

dantleech May 31, 2016 •

edited

wouterj commented May 29, 2016

dantleech May 29, 2016

dantleech commented Jun 5, 2016

dantleech Jun 5, 2016 •

edited

wouterj Jun 5, 2016

dantleech Jun 15, 2016 •

edited

dantleech commented Jun 5, 2016

wouterj Jun 5, 2016

dantleech commented Jun 15, 2016

dantleech commented Jun 20, 2016

wouterj Jun 20, 2016

Description Enhancer integration #29

Description Enhancer integration #29

Conversation

dantleech commented May 28, 2016

dantleech May 28, 2016

Choose a reason for hiding this comment

wouterj May 29, 2016

Choose a reason for hiding this comment

dantleech May 29, 2016

Choose a reason for hiding this comment

dbu May 31, 2016 via email

Choose a reason for hiding this comment

dantleech May 31, 2016 • edited

Choose a reason for hiding this comment

wouterj commented May 29, 2016

dantleech May 29, 2016

Choose a reason for hiding this comment

dantleech commented Jun 5, 2016

dantleech Jun 5, 2016 • edited

Choose a reason for hiding this comment

wouterj Jun 5, 2016

Choose a reason for hiding this comment

dantleech Jun 15, 2016 • edited

Choose a reason for hiding this comment

dantleech commented Jun 5, 2016

wouterj Jun 5, 2016

Choose a reason for hiding this comment

dantleech commented Jun 15, 2016

dantleech commented Jun 20, 2016

wouterj Jun 20, 2016

Choose a reason for hiding this comment

dantleech May 31, 2016 •

edited

dantleech Jun 5, 2016 •

edited

dantleech Jun 15, 2016 •

edited