-
Notifications
You must be signed in to change notification settings - Fork 56
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
Still works on PHP 7.1 #226
Conversation
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 7.1 be added to
coding-standard/.github/workflows/continuous-integration.yml
Lines 92 to 95 in b27f98f
php-version: | |
- "7.2" | |
- "7.3" | |
- "7.4" |
?
Should this be changed to 7.1
?
- "7.2" |
@greg0ire yes to both, added and works. |
Note to the person that merges this: you will have to mark "Test fixes (7.2)" as no longer required, and the new 7.1 jobs as required in the settings just before merging this. |
Thanks @beberlei ! |
Ah crap, this shouldn't have been merged in |
@greg0ire you could argue its a bugfix, as 7.1 support was wrongly removed :-) |
Yeah sure. I think there are no BC-breaks on master. Also, |
Restricting the 8.1 branch to 7.1 is an artificial constraint, the code works on 7.1 just fine.
#127 removed 7.1 support, but slevomat/coding-standard reintroduced 7.1 support with slevomat/coding-standard@b26cb5c#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34