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

Added Eloquent Model alias at ApplicationProvider #273

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eusonlito
Copy link

Related with issue #271

@alies-dev alies-dev added release:fix for PRs only (used by release-drafter) release:patch for PRs only (used by release-drafter) labels Jan 19, 2023
@eusonlito
Copy link
Author

@lptn The failed tests are my bad?

@alies-dev
Copy link
Collaborator

@eusonlito

The failed tests are my bad?

I don't think so. Codeception tests failed due to Psalm changes (a new output), Laravel projects tests failed due to a flag to find unused code. I'll try to fix both tomorrow (if there are no other volunteers), but it will be nice to add tests for your changes.

@eusonlito
Copy link
Author

@lptn

but it will be nice to add tests for your changes.

I don't know how codeception works 😥

@mr-feek
Copy link
Collaborator

mr-feek commented Jan 19, 2023

Thanks for the PR! 🤝

This seems reasonable to me.

However, I did just think of a slight edge case. Consider that Eloquent isn't aliased in the application at runtime in actual code, however the runtime code actually references Eloqouent as if it was an alias -- psalm would think that Eloquent is aliased and the code will work properly, rather than resulting in an error reported by psalm.

I really think this should be fixed upstream in ide-helper, but I think I'm okay with merging this as-is for now.

@alies-dev
Copy link
Collaborator

Hey @eusonlito
Do you still have this issue with \Eloquent? Do we need to work on this PR?

Is this PR related to the issue barryvdh/laravel-ide-helper#1352 ?

@alies-dev alies-dev self-requested a review March 12, 2024 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix for PRs only (used by release-drafter) release:patch for PRs only (used by release-drafter)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants