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

[HttpKernel] Check if "symfony/proxy-manager-bridge" package is installed #14266

Merged
merged 1 commit into from May 6, 2015

Conversation

hason
Copy link
Contributor

@hason hason commented Apr 8, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

@@ -662,7 +662,7 @@ protected function getContainerBuilder()
{
$container = new ContainerBuilder(new ParameterBag($this->getKernelParameters()));

if (class_exists('ProxyManager\Configuration')) {
if (class_exists('ProxyManager\Configuration') && class_exists('Symfony\Bridge\ProxyManager\LazyProxy\Instantiator\RuntimeInstantiator')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the first condition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think not. This PR solves a case when "symfony/framework-bundle" and "ocramius/proxy-manager" are installed. The first condition solves a case when "symfony/symfony" is installed and "ocramius/proxy-manager" not.

Copy link
Contributor

Choose a reason for hiding this comment

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

but I'm not sure If that makes any sense, if class RuntimeInstantiator exist then ProxyManager\Configuration exist also https://github.com/hason/symfony/blob/pm/src/Symfony/Bridge/ProxyManager/composer.json#L21 without checking it

Copy link
Member

Choose a reason for hiding this comment

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

@aitboudad you are only looking at the case of installing subtree packages. When installing the fullstack repo, the RuntimeInstantiator class always exists, as it is part of the fullstack repo. But this does not mean that the ProxyManager library is installed

Copy link
Contributor

Choose a reason for hiding this comment

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

now I understand :)

@hason
Copy link
Contributor Author

hason commented May 6, 2015

ping @aitboudad @fabpot

@aitboudad
Copy link
Contributor

👍

1 similar comment
@stof
Copy link
Member

stof commented May 6, 2015

👍

@fabpot
Copy link
Member

fabpot commented May 6, 2015

Thank you @hason.

@fabpot fabpot merged commit 43cc877 into symfony:2.3 May 6, 2015
fabpot added a commit that referenced this pull request May 6, 2015
…ge is installed (hason)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpKernel] Check if "symfony/proxy-manager-bridge" package is installed

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Commits
-------

43cc877 [HttpKernel] Check if "symfony/proxy-manager-bridge" package is installed
@hason hason deleted the pm branch May 18, 2015 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants