-
Notifications
You must be signed in to change notification settings - Fork 379
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
Symfony5 #1246
Symfony5 #1246
Conversation
e854bef
to
f8bf61e
Compare
Currently blocked by: php-enqueue/enqueue-dev#987 |
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.
looks correct to me.
we still have a lowest version build with 7.1 (hidden from the diff because unchanged), so we don't need to change composer.json for php version support
Would be good if we have it. Thx for your work!! |
composer.json
Outdated
"symfony/options-resolver": "^3.4|^4.2|^5.0", | ||
"symfony/process": "^3.4|^4.2|^5.0", | ||
"symfony/templating": "^3.4|^4.2|^5.0", | ||
"symfony/translation": "^3.4|^4.2|^5.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.
mmmm I think the bundle doesn't use any of "symfony/translation"
neither "symfony/asset"
php-enqueue/enqueue-dev#987 has been merged. edit: and a new release tagged. |
we need to allow enqueue |
Sure, although it doesn't seem they changed much php-enqueue/enqueue-dev@0.9.15...0.10.0 |
yeah, then i am hopeful we can simply allow that version and things will be fine. (i wanted to explain why we need to explicitly allow the version, did not want to imply that it would be a problem) |
enqueue-bundle dep needs to allow also LiipImagineBundle/composer.json Line 38 in f8bf61e
|
Hi! And happy holidays to everyone! I wanted to help push this forward (and I selfishly need this for a screencast!) so I took Michelle's work (as well as work from a different PR) and polished everything in #1256. That should now be ready. Thank you! |
Solved by #1256 |
I see test coverage officially fell but I am happy with our test coverage and merging despite that. |
Upgrading to Symfony 5 and adding php 7.4 support in travis while we're at it