Update PHPStan #1211
Update PHPStan #1211
Conversation
richardm-stripe
force-pushed
the
richardm-update-phpstan
branch
4 times, most recently
from
6bb1f2d
to
d02c3ce
Compare
richardm-stripe
force-pushed
the
richardm-update-phpstan
branch
from
645e117
to
96a4e46
Compare
| @@ -4,10 +4,6 @@ export PHPSTAN_VERSION := 0.12.59 | |||
| vendor: composer.json | |||
There was a problem hiding this comment.
Probably want to remove the PHPSTAN_VERSION export above.
| @@ -24,7 +24,8 @@ | |||
| "phpunit/phpunit": "^5.7 || ^9.0", | |||
| "php-coveralls/php-coveralls": "^2.1", | |||
| "squizlabs/php_codesniffer": "^3.3", | |||
| "friendsofphp/php-cs-fixer": "3.2.1 || 2.17.1" | |||
| "friendsofphp/php-cs-fixer": "3.2.1 || 2.17.1", | |||
| "phpstan/phpstan": "^1.2" | |||
There was a problem hiding this comment.
Will this cause issues installing the package with dev dependencies on earlier versions if we're saying it's not compatible with them?
There was a problem hiding this comment.
Yes. I think we should go with a policy of composer install is only supported on php 7.3+. I think this policy is fine because the only users of composer install are
- CI - which we have already handled with the
composer removeworkaround in this PR. - maintainers/contributors to stripe-php:
- I am happy to be restricted to php7.2+ when doing everyday development on stripe-php
- When I am explicitly trying to test compatibility things with <php7.2 it will be definitely an inconvenience to not be able to
composer installwithout manually working around, and beyond that it will be inconvenient to not be able to utilizephpstan/php-cs-fixer, but I think this inconvenience is far outweighed by the additional complexity it would take to continuously maintain compatibility with multiple versions of PHPStan and php-cs-fixer.
There was a problem hiding this comment.
Sounds good - let's leave a comment explaining this and steps for running <php 7.2 so that this doesn't come back to haunt us down the road once we've forgotten :)
| @@ -78,6 +78,8 @@ public function deleteDiscount($params = null, $opts = null) | |||
| $url = $this->instanceUrl() . '/discount'; | |||
| list($response, $opts) = $this->_request('delete', $url, $params, $opts); | |||
| $this->refreshFrom(['discount' => null], $opts, true); | |||
|
|
|||
| return $this; | |||
There was a problem hiding this comment.
This will need a codegen change right?
| @@ -66,8 +66,6 @@ class Quote extends ApiResource | |||
| * @param null|array|string $opts | |||
| * | |||
| * @throws \Stripe\Exception\ApiErrorException if the request fails | |||
| * | |||
| * @return \Stripe\File the created file | |||
There was a problem hiding this comment.
Flagging this will also need a codegen change.
richardm-stripe
force-pushed
the
richardm-update-phpstan
branch
3 times, most recently
from
8fd72d1
to
ca4c186
Compare
richardm-stripe
force-pushed
the
richardm-update-phpstan
branch
from
ca4c186
to
93308a4
Compare
Notify
r? @dcr-stripe
Summary
composer.jsonas a devDependency rather than being installed manually by the Makefile.