Navigation Menu

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

Add __toString method to ConnectionInterface #543

Closed
wants to merge 1 commit into from

Conversation

Daaarkling
Copy link

No description provided.

@nrk
Copy link
Contributor

nrk commented Aug 22, 2020

I'm closing this PR because, aside from missing any kind of explanation for requiring such change, it breaks the whole library since it simply moves the contract of requiring __toString() up in the inheritance tree but doesn't make any change to take care of classes that now have to implement this method to satisfy the new contract.

To explain thins a bit, only single node connections (Predis\Connection\NodeConnectionInterface) require __toString() as this method is used through the library to transparently cast connection parameters into an $ip:port pair.

Moving this requirement up to Predis\Connection\ConnectionInterface means to have this method implemented also for aggregate connections implementing Predis\Connection\AggregateConnectionInterface which doesn't make sense since they don't match a specific $ip:$port pair).

@nrk nrk closed this Aug 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants