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

shared = false attribute to services configuration to fix symfony >= 2.8 issue #26

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pedropapa
Copy link

@pedropapa pedropapa commented Mar 16, 2016

Shared: false attribute on services configuration is necessary because it prevents symfony from sharing the service scope among other classes (forcing a new class instance), it solves an issue on Reflection Factory when two or more bundles are added to the RegisterJsonSchemasPass compiler on bundle's build method. The scope sharing feature is default in symfony >= 2.8 and need to be disabled manually.

Note: it fixes #16

pedropapa added 2 commits Mar 16, 2016
…fix symfony >= 2.8 issue

Shared: false attribute on services configuration is necessary because it prevents symfony from sharing the service scope among other classes, it solves an issue on Reflection Factory when two or more bundles add the RegisterJsonSchemasPass compiler on bundle's build method. The scope sharing feature is default in symfony >= 2.8 and need to be disabled manually.
[UPDATE] Added shared = false attribute to services configuration to …
@phackwer
Copy link

phackwer commented Mar 16, 2016

Hey, I need this merge too. When will you accept it and make a release? Or did you abandon this project?

@phackwer
Copy link

phackwer commented Mar 16, 2016

@gquemener, are there plans to accept or reject? I'm facing the same problem here.

@gquemener
Copy link
Contributor

gquemener commented Mar 16, 2016

First of all, thank you!

I'm not sure this is compatible with symfony < 2.8 as the build matrix is not testing against different versions of symfony (but only the last one).

BTW, I'm no longer part of the Knp organization, and thus can't help you with the merging.

Let me ping a few of the folks for you @PedroTroller @docteurklein @AntoineLelaisant @Einenlum and @NiR-, that should do it 🍷

Cheers!

@pedropapa
Copy link
Author

pedropapa commented Mar 16, 2016

@gquemener You are right, I just tested the solution on Symfony 2.3 and a exception is thrown:

InvalidArgumentException: [ERROR 1866] Element '{http://symfony.com/schema/dic/services}service', attribute 'shared': The attribute 'shared' is not allowed.

I think it would need a major release to get this bundle running, but it is in dev mode still, so I suggest a branch called "symfony2.8".

@docteurklein
Copy link
Contributor

docteurklein commented Mar 17, 2016

@gquemener héhé, you don't have an excuse anymore: I just added you as a collaborator :D

@gquemener
Copy link
Contributor

gquemener commented Mar 17, 2016

Ok... Gimme some time, I will first change the build matrix to test it
against many sf versions!
Le 17 mars 2016 9:11 AM, "Florian Klein" notifications@github.com a
écrit :

@gquemener https://github.com/gquemener héhé, you don't have an excuse
anymore: I just added you as a collaborator :D


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#26 (comment)

@phackwer
Copy link

phackwer commented Mar 17, 2016

So far, I'm still waiting for this release, but seems like I'll have to fork until you accept the merge, because I have my deadlines here. Thanks for replying @gquemener

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

Successfully merging this pull request may close these issues.

4 participants