Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

moves the sonata admin description enhancer from resource component #76

Merged
merged 2 commits into from
Jan 27, 2017

Conversation

ElectricMaxxx
Copy link
Member

@ElectricMaxxx ElectricMaxxx commented Jan 27, 2017

fix symfony-cmf/resource#34
move sonata admin description enhancer.

@ElectricMaxxx ElectricMaxxx requested a review from wouterj January 27, 2017 12:41
@ElectricMaxxx
Copy link
Member Author

Now the php version changes are hitting me.

* file that was distributed with this source code.
*/

namespace Symfony\Cmf\Bundle\SonataPhpcrAdminIntegrationBundle\Enhancer;
Copy link
Member

Choose a reason for hiding this comment

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

I would rather put this in Description

Copy link
Member Author

Choose a reason for hiding this comment

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

i was unclear about the naming.

@@ -12,7 +12,7 @@
public="false">
<argument type="service" id="sonata.admin.pool" />
<argument type="service" id="router" />
<tag name="cmf_resource.description.enhancer" alias="sonata_admin" />
<tag name="cmf_resource.description.enhancer" alias="sonata_phpcr_admin_integration" />
Copy link
Member

Choose a reason for hiding this comment

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

I would still call this sonata_admin

Copy link
Member Author

Choose a reason for hiding this comment

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

As the master of resource-bundle still has that alias i got an collision. And when having a orm version with an description enhancer, we will have a collision too.

Copy link
Member Author

Choose a reason for hiding this comment

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

so at least i would keep the phpcr somewhere.

@@ -149,12 +149,16 @@ public function testEnhancerExists()
[
'SonataDoctrinePHPCRAdminBundle' => true,
'SonataAdminBundle' => true,
'JMSSerializerBundle' => true,
Copy link
Member

Choose a reason for hiding this comment

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

this is not needed here

Copy link
Member Author

Choose a reason for hiding this comment

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

but in AppKernel ..

public="false">
<argument type="service" id="sonata.admin.pool" />
<argument type="service" id="router" />
<tag name="cmf_resource.description.enhancer" alias="sonata_phpcr_admin_integration" />
Copy link
Member

Choose a reason for hiding this comment

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

Let's do sonata_phpcr_admin then.

*
* @internal
*/
class ResourceEnhancer implements DescriptionEnhancerInterface
Copy link
Member

Choose a reason for hiding this comment

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

All description enhancers describe resources. I would call this SonataEnhancer or the like, this specific enhancer implementation is about adding things that Sonata knows about our document (links, names, etc.)

fix tests

move classes

changes due to hints
@ElectricMaxxx
Copy link
Member Author

Changed the requested stuff, squashed it. Ready to merge.

@wouterj wouterj merged commit e0a0f15 into master Jan 27, 2017
@wouterj wouterj deleted the resource_enhancer branch January 27, 2017 15:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move admin enhancers out of here.
2 participants