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

Use PHPStan in Travis CI #787

Merged
merged 1 commit into from
Oct 30, 2019
Merged

Use PHPStan in Travis CI #787

merged 1 commit into from
Oct 30, 2019

Conversation

ob-stripe
Copy link
Contributor

r? @brandur-stripe @remi-stripe
cc @stripe/api-libraries @nauxliu

Add support for PHPStan, a static analyzer for PHP.

Because PHPStan requires PHP 7.1 or more recent, we cannot add it as a dev dependency in composer.json. I worked around this by setting up a Makefile to download PHPStan separately as a self-contained PHAR archive.

PHPStan offers different analysis levels, from 0 (loosest) to 7 (strictest). Right now I've set it at 1 and fixed most issues it reported. We can increase it and fix more issues in future PRs.

There are two issues I could not fix without some refactoring, so what I did was create a baseline file, which is basically the PHPStan equivalent of the .rubucop_todo.yml file. The phpstan-baseline make target can be used to regenerate the file.

.travis.yml Outdated Show resolved Hide resolved
@brandur-stripe
Copy link
Contributor

Left one minor comment, but nice one! Love that it's catching problems already.

ptal @ob-stripe

Copy link
Contributor

@remi-stripe remi-stripe left a comment

Choose a reason for hiding this comment

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

Approving. Exciting to see us add safeguards for this, thanks for doing the work on making this available! Left some minor comments to just get more context on the changes.

lib/HttpClient/CurlClient.php Show resolved Hide resolved
lib/ApiRequestor.php Show resolved Hide resolved
lib/StripeObject.php Show resolved Hide resolved
tests/Stripe/HttpClient/CurlClientTest.php Show resolved Hide resolved
@ob-stripe
Copy link
Contributor Author

Thanks again @nauxliu for getting the ball rolling with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants