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

Remove deprecated "get_parent_class" calls in Extendable trait. #153

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

bennothommo
Copy link
Member

This uses Reflection to instead determine the parent class and available magic methods to pass through to. In order to prevent infinite looping (which could've potentially been a problem before), it will also ignore any "extendable" classes when determining the parent.

Also fixed some tests that were using undefined class properties, and were also throwing deprecation errors.

Replaces #152

This uses Reflection to instead determine the parent class and available magic methods to pass through to. In order to prevent infinite looping (which could've potentially been a problem before), it will also ignore any "extendable" classes when determining the parent.

Also fixed some tests that were using undefined class properties, and were also throwing deprecation errors.

Replaces #152
@mjauvin
Copy link
Member

mjauvin commented Aug 14, 2023

Can't we use parent::class instead? But as far as I can see, get_parent_class($this) is still supported.

@LukeTowers
Copy link
Member

Is there any performance differences using the reflection API instead of get_class_parent?

@bennothommo
Copy link
Member Author

@mjauvin I can tell you that so far, I have not been able to find a way to use any sort of parent call in this context that doesn't end up in an infinite loop or throwing the error that came up in the #152 test cases. If someone smarter than me is able to figure it out, by all means. 🙂

@LukeTowers I haven't timed it yet, but I'm going to assume there's a small performance hit. But as above, it may be the only way to do it if we want to allow the magic methods of parent classes to still be fallen back upon.

@mjauvin
Copy link
Member

mjauvin commented Aug 14, 2023

@bennothommo and using get_parent_class($this) doesn't work either?

@bennothommo
Copy link
Member Author

@mjauvin It didn't when I tried it out, and it didn't in the test cases on the other PR I mentioned.

@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@LukeTowers
Copy link
Member

@bennothommo are you able to fix the code analysis error?

@bennothommo
Copy link
Member Author

@LukeTowers done

@LukeTowers LukeTowers merged commit d1b95f7 into develop Oct 19, 2023
8 checks passed
@LukeTowers LukeTowers deleted the fix/extendable-deprecations branch October 19, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants