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

Add static analysis #549

Merged
merged 2 commits into from
Mar 9, 2022
Merged

Add static analysis #549

merged 2 commits into from
Mar 9, 2022

Conversation

matthewtrask
Copy link
Contributor

so this way people can run these tools locally, before they hit CI.

@matthewtrask matthewtrask requested a review from Nyholm March 9, 2022 13:36
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Im not a big fan of this. I believe PHPstan and Psalm are tools, just like PHPStorm. All three will help you two write better code and none is integrated like PHPUnit.

I am the kind of programmer that always make 2-3 commits per pull request to fix the issues after CI is run. But I also know that other people would like to fix CS issues before they push.

So I am more like ¯\(ツ)/¯. Merge it if you like.
But let's remove it if/when it causes dependency issues.


FYI, I've installed these tool globally on my machine.

@matthewtrask
Copy link
Contributor Author

@Nyholm thats all fair! I tend to stick to by project installs for some reason. I will merge for the moment, but like you said if the dependencies get messy we can definitely remove it.

@matthewtrask matthewtrask merged commit 71f9862 into master Mar 9, 2022
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.

2 participants