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.

Sign up for GitHub

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

Jump to bottom

Create a service locator for clients to be used in various locations #419

Closed
curry684 opened this issue Apr 19, 2018 · 7 comments
Closed

Create a service locator for clients to be used in various locations #419

curry684 opened this issue Apr 19, 2018 · 7 comments
Assignees
@curry684
Labels
feature Introduces new functionality
Milestone
3.0

Comments

@curry684
Copy link
Collaborator

#409 (comment)

Given that it's a change in master only we'll now have to ensure we have a service locator so the commands and other explicit usages still work.
@curry684 curry684 added the feature Introduces new functionality label Apr 19, 2018
@curry684 curry684 added this to the 3.0 milestone Apr 19, 2018
@curry684 curry684 mentioned this issue Apr 19, 2018
Closed
@curry684 curry684 self-assigned this May 3, 2018
Basster added a commit to Basster/SncRedisBundle that referenced this issue Jun 6, 2018
@Basster
snc#419: Create a service locator for clients to be used in the comma…
479641c 
…nds.
Basster added a commit to Basster/SncRedisBundle that referenced this issue Jun 6, 2018
@Basster
snc#419: argh... tests...
f2aa780
@Basster
Copy link
Contributor

Basster commented Jun 7, 2018

@curry684 I've created #433, so the commands are working again. Feel free to point me to places where the service locator makes sense, too.

curry684 pushed a commit that referenced this issue Jun 19, 2018
@Basster @curry684
#419: Create a service locator for clients to be used in the commands. (
6db83c0 
#433)

* #419: Create a service locator for clients to be used in the commands.

* #419: argh... tests...

* Update env_profile.yaml

remove unrelated .yaml change

* Update env_profile.yaml

made REDIS_PROFILE a string again, as travis fails on PHP 7.1 and 7.2

* rename method

* follow test functions name convention.
@curry684
Copy link
Collaborator Author

Sorry for my late reply, I've been swamped in work so haven't had a chance really to look at this yet. Right now we're maintaining 2.x and 3.x is "coming some day".

@B-Galati
Copy link
Collaborator

@curry684 I guess this one can be closed no? Should we add some docs about it?

@curry684
Copy link
Collaborator Author

Nah think it's fine like this, I kept it open mainly as a reminder to look at it once we'd cleaned up for 3.x but given the overall direction I don't see a reason to keep it around.

@B-Galati
Copy link
Collaborator

@curry684 Do you think we should document this feature or it should be marked as internal?
I guess it can be documented but I would like to have your thoughts :-)

@curry684
Copy link
Collaborator Author

As far as I'm concerned it's an implementation detail. If library consumers wish to use similar functionality they can and should use service subscribers instead.

@B-Galati
Copy link
Collaborator

Makes sense, agree! Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@curry684 curry684

Labels
feature Introduces new functionality
Projects
None yet
Milestone
3.0
Development

No branches or pull requests
3 participants
@B-Galati @Basster @curry684