Skip to content

Removed the warning when installing extensions for PHP 7#880

Merged
c9s merged 1 commit into
phpbrew:masterfrom
morozov:no-php7-warning
Jun 18, 2017
Merged

Removed the warning when installing extensions for PHP 7#880
c9s merged 1 commit into
phpbrew:masterfrom
morozov:no-php7-warning

Conversation

@morozov

@morozov morozov commented Jun 14, 2017

Copy link
Copy Markdown
Contributor

While PHP 5.6 only receives security updates and PHP 7 overall now represents over 50% of installations, the warning in the question seems no more relevant.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 51.459% when pulling 1b82254 on morozov:no-php7-warning into 5f19573 on phpbrew:master.

3 similar comments
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 51.459% when pulling 1b82254 on morozov:no-php7-warning into 5f19573 on phpbrew:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 51.459% when pulling 1b82254 on morozov:no-php7-warning into 5f19573 on phpbrew:master.

@coveralls

coveralls commented Jun 14, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.02%) to 51.459% when pulling 1b82254 on morozov:no-php7-warning into 5f19573 on phpbrew:master.

@coveralls

coveralls commented Jun 14, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.1%) to 51.576% when pulling c2eabb2 on morozov:no-php7-warning into 5f19573 on phpbrew:master.

@jhdxr

jhdxr commented Jun 15, 2017

Copy link
Copy Markdown
Member

LGTM

the warning that the version of extensions should match the version of php is applied to all php versions, not only 7.x. I think simpily removing it or changing it to a notice for all versions is ok.

@morozov

morozov commented Jun 15, 2017

Copy link
Copy Markdown
Contributor Author

@jhdxr I would just remove it. If the extension is successfully installed, there's nothing to warn/notify above. If it fails, there may be a thousand reasons to that besides version incompatibility. Without the actual logic which detects compatibility, this message doesn't add any value.

@c9s c9s merged commit 29e0a11 into phpbrew:master Jun 18, 2017
@c9s

c9s commented Jun 18, 2017

Copy link
Copy Markdown
Member

looks good, I think most of the users are now installing extensions for php7 not for php5. so I think this is good to be merged. Or it will be too annoying if there are too many messages.

@morozov morozov deleted the no-php7-warning branch June 18, 2017 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants