add vimeo/psalm, update to phpunit 7.x, change require-dev to require…#22
add vimeo/psalm, update to phpunit 7.x, change require-dev to require…#22DavidGoodwin wants to merge 5 commits intowdalmut:masterfrom
Conversation
|
can you upgrade travis configuration to php7.4 so the test suit will run? |
possibly done .... (I'm commenting before I've checked if travis is happy ... it may not like php7.2 or 5.6 now, I'm not sure) |
wdalmut
left a comment
There was a problem hiding this comment.
i prefer to drop the vimeo/psalm support, i think that less dependency is better
.travis.yml
Outdated
| php: | ||
| - 7.4 | ||
| - 7.2 | ||
| - 5.6 |
There was a problem hiding this comment.
can i also ask to drop php5.6 from travis-ci?
composer.json
Outdated
| "mikey179/vfsstream": "1.4.*" | ||
| "php": ">=7.2", | ||
| "phpunit/phpunit": "~7", | ||
| "mikey179/vfsstream": "1.4.*", |
There was a problem hiding this comment.
can you upgrade to the latest 1.6.*?
There was a problem hiding this comment.
done (changed to ^1.4)
| "php": ">=7.2", | ||
| "phpunit/phpunit": "~7", | ||
| "mikey179/vfsstream": "1.4.*", | ||
| "vimeo/psalm" : "*" |
There was a problem hiding this comment.
psalm is a dev only dependency - it won't affect users of the package.
it will help make sure you've not added errors into the codebase (E.g. wrong parameters, missing parameter, incorrect doc types etc).
| "vimeo/psalm" : "*" | ||
| }, | ||
| "scripts": { | ||
| "psalm" : "@php vendor/bin/psalm", |
There was a problem hiding this comment.
adding it helps the code quality. I don't see why you'd want to drop it.
Normal users of the package will not install psalm as a dependency.
| @@ -0,0 +1,15 @@ | |||
| <?xml version="1.0"?> | |||
There was a problem hiding this comment.
can you drop psalm support? i think that is better to have less..
Upgrade to newer PHPUnit version
Add vimeo/psalm for possible static analysis checks