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 phpstan on staged PHP source files #5424

Merged
merged 1 commit into from Sep 23, 2020

Conversation

pierlon
Copy link
Contributor

@pierlon pierlon commented Sep 23, 2020

Summary

This PR is a followup from #4441, which runs phpstan on staged PHP files within the includes or src folders.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@pierlon pierlon added the Infrastructure Changes impacting testing infrastructure or build tooling label Sep 23, 2020
@pierlon pierlon added this to the v2.1 milestone Sep 23, 2020
@pierlon pierlon self-assigned this Sep 23, 2020
@google-cla google-cla bot added the cla: yes Signed the Google CLA label Sep 23, 2020
@github-actions
Copy link
Contributor

Plugin builds for 3e4af0c are ready 🛎️!

@pierlon pierlon added the WS:Core Work stream for Plugin core label Sep 23, 2020
@pierlon pierlon added this to Ready for Review in Ongoing Sep 23, 2020
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

Excellent. Thank you.

@westonruter westonruter merged commit d61923e into develop Sep 23, 2020
@westonruter westonruter deleted the enhancement/static-analysis-on-staged-files branch September 23, 2020 23:41
@westonruter westonruter moved this from Ready for Review to Ready for QA in Ongoing Sep 23, 2020
@schlessera
Copy link
Collaborator

Did you test whether this works as intended? I can't imagine static analysis being useful is not run across the whole codebase. Contrary to PHPCS, the whole point of PHPStan is to look at the whole and reason about whether the flow across the codebase makes sense everywhere. If you just look at one file, you cannot really say much about typing consistency and similar issues.

@westonruter
Copy link
Member

I did test. It caught an issue with a staged file.

@schlessera
Copy link
Collaborator

This goes counter to what the author of PHPStan recommends, though: https://medium.com/@ondrejmirtes/from-minutes-to-seconds-massive-performance-gains-in-phpstan-163be88d1519

@pierlon
Copy link
Contributor Author

pierlon commented Sep 24, 2020

Ah I did not know that. So we can configure phpstan to run via the pre-commit hook then, instead of on staged files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA Infrastructure Changes impacting testing infrastructure or build tooling WS:Core Work stream for Plugin core
Projects
Ongoing
  
QA Passed
Development

Successfully merging this pull request may close these issues.

None yet

3 participants