You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Protected/Private Attributes marked methods are not included in the model docBlock #1293
While tinkering a bit with how Laravel handles these methods I found that we can get all methods even non-public by using (new ReflectionClass($instance))->getMethods() instead of get_class_methods()
@mridul89 seems I've forgotten about it; my first reaction: 0 tests. Probably that's the reason I wanted to queue it for later and then forgot about it…
In general, if you add a feature or fix a bug, you write tests for it to prove that it works since the existing tests won't point that out.
It's also a good idea to mention the issue you're fixing in the description of the PR. Since this issue wasn't mentioned in your PR I didn't see it and therefore didn't know it existed.
Activity
viicslen commentedon Jan 5, 2022
While tinkering a bit with how Laravel handles these methods I found that we can get all methods even non-public by using
(new ReflectionClass($instance))->getMethods()instead ofget_class_methods()pindab0ter commentedon Feb 10, 2022
I would love to see this improvement made to be able to adhere to the recommended code style.
mridul89 commentedon Apr 7, 2022
I created PR for this around a month ago- #1325
mfn commentedon Apr 7, 2022
@mridul89 seems I've forgotten about it; my first reaction: 0 tests. Probably that's the reason I wanted to queue it for later and then forgot about it…
mridul89 commentedon Apr 7, 2022
@mfn Sorry about that, i thought the existing tests would be fine since this is a bugfix.
I pushed another commit on the PR #1325 now.
pindab0ter commentedon Apr 7, 2022
In general, if you add a feature or fix a bug, you write tests for it to prove that it works since the existing tests won't point that out.
It's also a good idea to mention the issue you're fixing in the description of the PR. Since this issue wasn't mentioned in your PR I didn't see it and therefore didn't know it existed.