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

[Async] [DI] Make resolve_cache_processor a public service #1043

Merged
merged 1 commit into from Feb 16, 2018

Conversation

silverbackdan
Copy link
Contributor

liip_imagine.async.resolve_cache_processor should be public or an exception is thrown when sending enqueue command.

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

I had an exception about this service needing to be public using SF4. All services are private by default now. Not sure where it was used and if dependency injection through the __construct method would be preferred but wanted to submit this PR as a quick easy fix which shouldn't result in any breaking changes with older symfony versions either.

liip_imagine.async.resolve_cache_processor should be public or an exception is thrown when sending enqueue command.
@robfrawley
Copy link
Collaborator

@makasim This involves the async functionality you almost exclusively wrote yourself; is this how you want to handle the situation for Symfony 4.x compatibility?

@robfrawley
Copy link
Collaborator

@makasim ping

@robfrawley robfrawley changed the title Set as public - resolve_cache_processor [Async] [DI] Make resolve_cache_processor a public service3 Feb 15, 2018
@robfrawley robfrawley changed the title [Async] [DI] Make resolve_cache_processor a public service3 [Async] [DI] Make resolve_cache_processor a public service Feb 15, 2018
@makasim makasim merged commit 2eafbe8 into liip:2.0 Feb 16, 2018
@makasim
Copy link
Collaborator

makasim commented Feb 16, 2018

This how it should be, at least for now since we still support SF2,3,4

@silverbackdan silverbackdan deleted the patch-1 branch February 16, 2018 09:39
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