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

[Dependency Injection] Change imagine controller from private service to public service #1019

Merged

Conversation

michaelperrin
Copy link
Contributor

@michaelperrin michaelperrin commented Dec 1, 2017

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

This fixes compatibility with Symfony 3.4+ where services are private by default, and avoids errors like this:

The "liip_imagine.controller" service or alias has been removed or inlined when the container was compiled.

Make liip_imagine.controller service public (Symfony 3.4 / 4.0 compatibility)

This fixes compatibility with Symfony 3.4+ where services are private by default.
@michaelperrin michaelperrin changed the title Make liip_imagine.controller service public Make liip_imagine.controller service public (Symfony 3.4 / 4.0 compatibility) Dec 1, 2017
@lsmith77
Copy link
Contributor

lsmith77 commented Dec 1, 2017

thank you!

@lsmith77 lsmith77 merged commit 67fc8be into liip:2.0 Dec 1, 2017
@michaelperrin
Copy link
Contributor Author

Thanks for merging so fast @lsmith77!
I can now update my project to Symfony 4 :)

@lsmith77
Copy link
Contributor

lsmith77 commented Dec 1, 2017

let me know how it goes ..

@robfrawley what is the plan for a 2.0 release or should we actually also try and make 1.x compatible with Symfony 3.4/4.0 since it seems like there are still a bunch of open tasks for 2.0 https://github.com/liip/LiipImagineBundle/issues?q=is%3Aopen+is%3Aissue+milestone%3A2.0.0

@michaelperrin
Copy link
Contributor Author

It works fine actually.
I am using it on a small side project in production that I upgraded to Symfony 4 and where thumbnails are generated with LiipImagineBundle (here it is: http://www.randonavigo.fr).

But the dependency is on 2.0.x-dev version. So this is a good question to ask about 1.x compatibility with Symfony 3.4 / 4.0.

@robfrawley
Copy link
Collaborator

robfrawley commented Jan 19, 2018

@lsmith77 Well, 2.x 1.x needs to be merged (and conflict-resolved) with 1.x 2.x, which is a bit behind ATM. I would recommend we leave 1.x as-is (with bug fixes, but no major new features moving forward) and ensure 2.x is compatible moving forward, especially as 2.x not having a release yet allows us to remain fluid with dependencies and ensure we have a solid release for the next major release.

@lsmith77
Copy link
Contributor

@robfrawley yeah I agree. what has me a bit concerned is how long it will take us to implement all the refactorings that break BC so that we can make a first 2.0 release, since until then I guess we will not have a stable version compatible with Symfony 4 ..

@robfrawley
Copy link
Collaborator

Interesting you should bring up the work required to bring 2.x in-line with the latest Symfony 4.x conventions: I'm in the process of migrating the first of my clients to Symfony 4.x, and they happen to utilize this bundle. Generally, a large portion of my contributions here are a direct result of changes/improvements I make during such projects, so I should have an excuse to direct some time toward just what you mentioned. Two questions:

  • What kind of time-frame do we want to shoot for to release 2.x?
  • Our 2.x dependency requirement for Symfony will be 3.4+/4.x and for PHP will be 7.1, correct?

Regarding the remaining open tasks for 2.x, I'd only marked one as a blocker (#806) and we should likely review them all and prioritize/cut as nessissary. Can we get @lsmith77 @antoligy @cedricziel to take a moment to review the open 2.x tasks and either upvote (:+1:) or downvote (:-1:) the main issue summaries for the 2.x tasks (these up/down votes are to indicate importance for inclusion in the 2.x release, not to indicate your agreeance/disagreeance with the issue itself).

@lsmith77
Copy link
Contributor

well from the looks if it, I guess #806 is also the only one that will lead to BC breaks .. so the others could be done in 2.1 I guess

@robfrawley robfrawley changed the title Make liip_imagine.controller service public (Symfony 3.4 / 4.0 compatibility) [Dependency Injection] Change imagine controller from private service to public service Mar 12, 2018
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.

None yet

3 participants