Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[3.0] [FrameworkBundle] removed request service occurrences.#12460
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
hhamon commentedNov 11, 2014
Q | A |
---|---|
Bug fix? | no |
New feature? | no |
BC breaks? | yes |
Deprecations? | no |
Tests pass? | no |
Fixed tickets | ~ |
License | MIT |
Doc PR | ~ |
piotrpasich commentedNov 12, 2014
👍 |
1 similar comment
👍 |
@@ -56,7 +56,7 @@ | |||
</service> | |||
<service id="templating.asset.request_aware_package" class="Symfony\Component\Templating\Asset\PackageInterface" factory-service="templating.asset.package_factory" factory-method="getPackage" abstract="true"> | |||
<argument type="service" id="request"strict="false" /> | |||
<argument type="expression"strict="false">service('request_stack').getCurrentRequest()</argument> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
strict=false
is not needed anymore, given that there is no scope issue anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
thanks I'm gonna change it.
6fa6e98
to1933691
Compareb120c3c
to1b18f88
CompareTests are finally green! Ping @symfony/deciders @symfony/mergers. |
@@ -44,7 +44,7 @@ | |||
</service> | |||
<service id="templating.asset.path_package" class="%templating.asset.path_package.class%" abstract="true"> | |||
<argument type="service" id="request" /> | |||
<argument type="expression">service('request_stack').getCurrentRequest()</argument> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think usinggetMasterRequest()
is more correct here (same below).
1b18f88
to74d7cc3
Compare@fabpot done ;) |
<service id="service_container" synthetic="true" /> | ||
<service id="kernel" synthetic="true" /> | ||
<service id="filesystem" class="%filesystem.class%"></service> | ||
<service id="filesystem" class="%filesystem.class%"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Missing space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Fixed thanks.
74d7cc3
to5fe3e61
Compare5fe3e61
to2120554
CompareThank you@hhamon. |
…ences. (hhamon)This PR was merged into the 3.0-dev branch.Discussion----------[3.0] [FrameworkBundle] removed request service occurrences.| Q | A| ------------- | ---| Bug fix? | no| New feature? | no| BC breaks? | yes| Deprecations? | no| Tests pass? | no| Fixed tickets | ~| License | MIT| Doc PR | ~Commits-------2120554 Removed request service occurrences.
<factory service="templating.asset.package_factory" method="getPackage" /> | ||
<argument type="service" id="request" strict="false" /> | ||
<service id="templating.asset.request_aware_package" class="Symfony\Component\Templating\Asset\PackageInterface" factory-service="templating.asset.package_factory" factory-method="getPackage" abstract="true"> | ||
<argument type="expression">service('request_stack').getMasterRequest()</argument> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This will break if the master request is not available (in the CLI for instance). It would be better to inject the RequestStack itself.