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

[5.6] Fix ConnectionInterface methods #25092

Merged
merged 1 commit into from
Aug 5, 2018
Merged

Conversation

raftalks
Copy link
Contributor

@raftalks raftalks commented Aug 5, 2018

Declaration of Illuminate\Database\Connection class methods must be compatible with Illuminate\Database\ConnectionInterface

Does not require any change in the tests.
Doesn't break any existing feature.

Declaration of Illuminate\Database\Connection class methods must be compatible with Illuminate\Database\ConnectionInterface
@laurencei
Copy link
Contributor

Probably should just target 5.7 that is due out soon, since your technically changing an interface file...

@taylorotwell taylorotwell merged commit 7a5b58b into laravel:5.6 Aug 5, 2018
@raftalks raftalks deleted the patch-7 branch August 5, 2018 06:46
ntzm added a commit to ntzm/framework that referenced this pull request Aug 5, 2018
@GrahamCampbell GrahamCampbell changed the title Fix ConnectionInterface methods [5.6] Fix ConnectionInterface methods Aug 5, 2018
@GrahamCampbell
Copy link
Member

This fix was already made to 5.7 quite a while ago. I've reverted this on 5.6 because it is a major breaking change. It remains on 5.7+.

@raftalks
Copy link
Contributor Author

raftalks commented Aug 5, 2018

I came across with an issue on 5.6 while using decorated Connection class where I am overriding a method, and since I noticed my implementation does comply with the interface it however fails to inherit the connection class. The parameter was added to version 5.6 according to this PR 8afc151

🙄

@X-Coder264
Copy link
Contributor

@raftalks Nope, according to that commit 8afc151 it was added to 5.3.27 -> #16625

@raftalks
Copy link
Contributor Author

raftalks commented Aug 5, 2018

Sorry, my bad, didn’t really check when it was added as I was on mobile. Even though it’s been added and still do exist in 5.6 with a method that contracts to the interface, shouldn’t they comply?

@GrahamCampbell
Copy link
Member

shouldn’t they comply?

No as that it a major breaking change to a contract.

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.

5 participants