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

Run static code analysis on GitHub Actions #1336

Closed
wants to merge 2 commits into from
Closed

Run static code analysis on GitHub Actions #1336

wants to merge 2 commits into from

Conversation

fbourigault
Copy link
Contributor

@fbourigault fbourigault commented Jan 4, 2021

Q A
Branch? 2.x
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

As discussed with @dbu, here is a PR to replace most of Scrutinizer work with static code analysis.

I choose PHPStan as I use it daily at work and it's the tool used by SonataAdminBundle which is my main source of inspiration for all my GitHub Actions PRs.

@coveralls
Copy link

coveralls commented Jan 4, 2021

Coverage Status

Coverage remained the same at 83.645% when pulling d68b9be on fbourigault:github-actions-phpstan into db366d2 on liip:2.x.

@fbourigault fbourigault marked this pull request as ready for review January 5, 2021 23:19
@fbourigault
Copy link
Contributor Author

fbourigault commented Jan 6, 2021

With current implementation, phpstan can be installed with a newer version in an unrelated PR.

This could lead to unrelated build failures.

What are our options to lock phpstan to a given version?

EDIT:
I looked at SonataAdminBundle and they don't do anything. We can merge this one and improve things if it gets annoying.

@lsmith77
Copy link
Contributor

there seems to be a minor conflict.

@lsmith77
Copy link
Contributor

for a sub 1.0 dependency on a require-dev I am totally fine locking for a specific version.

Copy link
Member

@dbu dbu left a comment

Choose a reason for hiding this comment

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

sorry for the late reply. looks good to me, thanks a lot for the work! didn't know about the phpstan stub functionality, that is quite neat (i always made it ignore the warnings about unknown classes, with a comment that this was legacy support. much better and safer with the stubs thing!)

i will rebase the branch to solve conflicts and then merge.

@dbu dbu mentioned this pull request Oct 5, 2021
@dbu
Copy link
Member

dbu commented Oct 5, 2021

thanks! i rebased and solved conflicts in #1390 but unfortunately a couple of failures now show

@dbu dbu closed this in #1390 Oct 6, 2021
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.

4 participants