-
Notifications
You must be signed in to change notification settings - Fork 117
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
make the bundle Symfony 3 compatible, bump Symfony dependency to 2.6 #184
Conversation
@craue great work! is there any ETA for 3.0.0? thx |
@patie, I want to wait at least until Symfony 2.8 is released to ensure the bundle is fully compatible without triggering any deprecation notices. |
0ff33ce
to
aaf70ed
Compare
f5012af
to
cdb074f
Compare
Tests pass, the jobs only fail at the Coveralls data upload due to the outdated (and obviously unmaintained) satooshi/php-coveralls component. Ready for review. /ping @stof 😏 |
@@ -37,6 +38,15 @@ public function registerBundles() { | |||
|
|||
public function registerContainerConfiguration(LoaderInterface $loader) { | |||
$loader->load($this->config); | |||
|
|||
if (class_exists('Symfony\Component\Asset\Package')) { | |||
// enable assets to avoid fatal error "Call to a member function needsEnvironment() on a non-object in vendor/twig/twig/lib/Twig/Node/Expression/Function.php on line 25" with Symfony 3.0 |
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.
do you really need it ? Do you depend on AsseticBundle for your integration tests ?
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 don't need Assetic for the tests, but got an error regarding "asset" being undefined. When trying locally now, it's gone. 😒 I'll remove this code.
…ymfony 3.0 compatibility
… `RequestStack` is now used by default
…vely bumping Symfony dependency to 2.4
cdb074f
to
5f2fd76
Compare
env: SYMFONY_VERSION="2.4.*" SYMFONY_DEPRECATIONS_HELPER="weak" | ||
- php: 5.6 | ||
env: SYMFONY_VERSION="2.5.*" SYMFONY_DEPRECATIONS_HELPER="weak" | ||
- php: 5.6 | ||
env: SYMFONY_VERSION="2.6.*" SYMFONY_DEPRECATIONS_HELPER="weak" | ||
- php: 5.6 | ||
env: SYMFONY_VERSION="2.8.*" SYMFONY_DEPRECATIONS_HELPER="weak" |
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.
2.8 should not allow weak deprecations (otherwise, the code is not Symfony 3 ready)
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.
Ah right, I forgot to remove it from that line. Done.
5f2fd76
to
4109e69
Compare
Worked around the issue with satooshi/php-coveralls making the build completely green now. 😎 |
8f8f131
to
41687c0
Compare
41687c0
to
c67560f
Compare
Going to merge this soon if there are no further objections. |
make the bundle Symfony 3 compatible, bump Symfony dependency to 2.6
Thank you for your feedback, @stof. |
Two BC breaking changes are needed to make the bundle Symfony 3 compatible:
request_stack
service instead ofrequest
,Because
RequestStack
class was introduced by Symfony 2.4 andrequest
into request-scoped services,support for Symfony 2.3 needs to be dropped. Currently, Symfony 2.4 and 2.5 aren't maintained anymore, so it should be fine to support only at least 2.6.