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 a PhpStan shortcut to evaluate PR changes & moved PhpStan files #1146

Merged
merged 2 commits into from
Jul 30, 2021

Conversation

Koen1999
Copy link
Contributor

This makes it easier to review PRs and the main directory is a bit cleaner

@tomudding
Copy link
Member

Quick question, phpstan/phpstan-baseline.neon has ignored errors. Is this intentional? Because I would think we should fix those "errors".

@Koen1999
Copy link
Contributor Author

Quick question, phpstan/phpstan-baseline.neon has ignored errors. Is this intentional? Because I would think we should fix those "errors".

Yep, this is intentional. Laminas returns an interface that does not very accurately describe the object being returned. I actually created an issue for this hoping that it will be improved in a later version. Perhaps with a new interface that has more of the methods that we use. (laminas/laminas-mvc#77)
The errors from PhpStan which we ignore aren't actually errors. It could be nice to add checks, but this would be quite some work that doesn't add functionality.

@Koen1999
Copy link
Contributor Author

@tomudding can we merge this so we can start using it? :)

Copy link
Member

@tomudding tomudding left a comment

Choose a reason for hiding this comment

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

Looks to be working. But we may have to be careful with the git checkout --detach master, as not every branch is guaranteed to be based on the master branch.

@tomudding tomudding merged commit 239d5c2 into GEWIS:master Jul 30, 2021
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.

2 participants