-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Allow PHP ^8.0, add Github Actions #20
Conversation
(I'm not the owner of this repo) Hi, you have a much higher chance of merging a PR if you create two separate ones, each for separate concern this PR includes. |
Hi, thank you for the suggestion. I am aware of that. The reason why it is in single PR is the response time of the Travis CI (sometimes takes hours). The scope of |
See Github Actions executed in the fork scope: trejjam/roave-no-floaters#1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshuaslaney, please stop. This is not how things works... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Overall, it's a major improvement, and moves one of the last pending libraries off travis-ci, thanks @trejjam!
Checking builds, then merging :-)
|
||
strategy: | ||
matrix: | ||
php-version: ["7.4"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO to be tested only with 8.0
|
||
strategy: | ||
matrix: | ||
php-version: ["7.4"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO to be tested only with 8.0
.PHONY: install qa cs phpstan tests mutation-tests | ||
|
||
install: | ||
composer update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be composer install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense. I totally overlooked it. Thank you for the review!
@@ -5,16 +5,16 @@ | |||
"MIT" | |||
], | |||
"require": { | |||
"php": "^7.2", | |||
"php": "^7.4 | ^8.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
is deprecated - should be ||
Hi,
this PR adds support for PHP 8.0 in composer.json. And it also replaces Travis with Github Actions (based on Contributte Actions).