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

Check for traits recursively #1216

Merged
merged 5 commits into from Jun 20, 2021

Conversation

daniel-de-wit
Copy link
Contributor

@daniel-de-wit daniel-de-wit commented Apr 30, 2021

Summary

Parent traits are not checked when checking for HasFactory or Macroable traits.

E.g; My models extend an AbstractModel which implements the HasFactory trait, but no documentation is added.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds legit to me on quick thought.

Can you add a test, please?

@daniel-de-wit
Copy link
Contributor Author

@mfn I've added a test 👍

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM + a small change suggestion (because class_uses_recursive takes care of this already)

Thanks, can you also add a changelog?

src/Console/ModelsCommand.php Outdated Show resolved Hide resolved
daniel-de-wit and others added 2 commits April 30, 2021 20:36
Co-authored-by: Markus Podar <markus@fischer.name>
@daniel-de-wit
Copy link
Contributor Author

@mfn I've updated the changelog, not sure if this is what you meant?

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniel-de-wit yes, thank you, LGTM!

/cc @barryvdh

@mfn
Copy link
Collaborator

mfn commented May 2, 2021

The failed test is a result of composer timeout; I'd assume it's temporary thing and the build is fine.

@mfn
Copy link
Collaborator

mfn commented May 3, 2021

Hmm, we've the same failing jobs now in #1185 too

mfn added a commit to mfn/laravel-ide-helper that referenced this pull request May 3, 2021
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh#1076

See also:
- barryvdh#1216 (comment)
- barryvdh#1185 (comment)
@mfn mfn mentioned this pull request May 3, 2021
9 tasks
barryvdh pushed a commit that referenced this pull request Jun 20, 2021
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of #1076

See also:
- #1216 (comment)
- #1185 (comment)
@barryvdh barryvdh closed this Jun 20, 2021
@barryvdh barryvdh reopened this Jun 20, 2021
@barryvdh barryvdh merged commit e71f3d5 into barryvdh:master Jun 20, 2021
@daniel-de-wit daniel-de-wit deleted the check-traits-recursively branch June 20, 2021 08:29
fatihdirikman added a commit to fatihdirikman/Laravel-IDE-Helper that referenced this pull request Jan 7, 2022
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh/laravel-ide-helper#1076

See also:
- barryvdh/laravel-ide-helper#1216 (comment)
- barryvdh/laravel-ide-helper#1185 (comment)
renaforsberg824 added a commit to renaforsberg824/ide-helper-laravel-developer that referenced this pull request Oct 5, 2022
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh/laravel-ide-helper#1076

See also:
- barryvdh/laravel-ide-helper#1216 (comment)
- barryvdh/laravel-ide-helper#1185 (comment)
lisadeloach63 added a commit to lisadeloach63/ide-helper-reso-laravel that referenced this pull request Oct 7, 2022
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh/laravel-ide-helper#1076

See also:
- barryvdh/laravel-ide-helper#1216 (comment)
- barryvdh/laravel-ide-helper#1185 (comment)
sadafrangian3 pushed a commit to sadafrangian3/ide-helper-laravel that referenced this pull request Oct 18, 2022
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh/laravel-ide-helper#1076

See also:
- barryvdh/laravel-ide-helper#1216 (comment)
- barryvdh/laravel-ide-helper#1185 (comment)
smile1130 added a commit to smile1130/laravel-IDE that referenced this pull request Jun 16, 2023
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh/laravel-ide-helper#1076

See also:
- barryvdh/laravel-ide-helper#1216 (comment)
- barryvdh/laravel-ide-helper#1185 (comment)
d3v2a pushed a commit to d3v2a/laravel-ide-helper that referenced this pull request Feb 16, 2024
IMHO it's impractical to expect this package to have to work with the
"first release of Laravel for any major release" even when there are
already sever more releases: no one is/should stick to that version.

This is a re-submit of barryvdh#1076

See also:
- barryvdh#1216 (comment)
- barryvdh#1185 (comment)
d3v2a pushed a commit to d3v2a/laravel-ide-helper that referenced this pull request Feb 16, 2024
* Check for traits recursively

* Remove autoload bool from class_uses_recursive

* Add test

* Update src/Console/ModelsCommand.php

Co-authored-by: Markus Podar <markus@fischer.name>

* Updated changelog

Co-authored-by: Markus Podar <markus@fischer.name>
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.

None yet

3 participants