-
Notifications
You must be signed in to change notification settings - Fork 51
feat(psalm): Add all stubs #1875
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
Conversation
313d9be
to
7f2facc
Compare
Why does it need such a recent PHP?
|
Yeah it's a bit weird that the version requirements are so strict (maybe for security reasons, but that's not really psalms task?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if the PHP version thing is solved and I can run the analysis locally.
Also the CI will have trouble I expect as it does not always have the last PHP?
And would be better if the IQueryBuilder thing is properly fixed as well.
Not gonna block this PR though, it’s still a step forward.
I'll try to improve this PR, but I already gave up on the IQueryBuilder thing because there is no trace of what is wrong 🤷♀️ |
7f2facc
to
0fce851
Compare
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
Signed-off-by: provokateurin <[email protected]>
0fce851
to
dc1dc88
Compare
I tried to fix a bunch of the errors that were uncovered by adding the proper stubs, but didn't manage to fix all of them since I don't understand everything yet.
I also don't understand why psalm doesn't properly understand that CoreQueryBuilder implements IQueryBuilder, but the change makes it work 🤷♀️