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.7] Add relationship getters #26607

Merged
merged 1 commit into from Nov 25, 2018
Merged

[5.7] Add relationship getters #26607

merged 1 commit into from Nov 25, 2018

Conversation

@staudenmeir
Copy link
Contributor

@staudenmeir staudenmeir commented Nov 24, 2018

Some of the relationship properties don't yet have their own getters, so there's no (good) way to access them from the outside.

@antonkomarev
Copy link
Contributor

@antonkomarev antonkomarev commented Nov 24, 2018

Such changes should be targeted to master branch and not stable 5.7.
Could you provide any examples why do you need to have access to this properties?

@deleugpn
Copy link
Contributor

@deleugpn deleugpn commented Nov 24, 2018

I don't think there's enough cause for targeting master on these changes. Although I am curious as to the use-case for it.

@staudenmeir
Copy link
Contributor Author

@staudenmeir staudenmeir commented Nov 24, 2018

@antonkomarev Why the master branch? There are no breaking changes.

I ran into this while working on one of my packages. Users can create a relationship by chaining multiple of their existing relationships. To implement this, I have to access all the relationships' keys. At the moment, I'm using the Closure::bind() workaround to get the protected properties.

My package aside, I think those properties should be accessible. There might be other use cases.

@taylorotwell taylorotwell merged commit 1ca55a1 into laravel:5.7 Nov 25, 2018
2 checks passed
@staudenmeir staudenmeir deleted the relation-getters branch Nov 25, 2018
fkeloks added a commit to fkeloks/framework that referenced this issue Nov 25, 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants