-
-
Notifications
You must be signed in to change notification settings - Fork 130
[WIP] Update .travis.yml for PHP 7 #45
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
Conversation
ffc7afa
to
acc8171
Compare
@@ -298,7 +298,7 @@ private function unsubscribeStreamEvent($stream, $flag) | |||
*/ | |||
private function createTimerCallback() | |||
{ | |||
$this->timerCallback = function ($_, $_, $timer) { | |||
$this->timerCallback = function ($_, $__, $timer) { |
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.
I see why we would want this change, but for the reference only: Afaict this line is currently untested because the libevent extension does not (currently) compile under PHP 7, right?
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.
Yep, right. The code crashes because of this line when you try to include
this class. That's something that static analysis tools can stumble upon.
So this fixes that. But there's not actually a way to run this
implementation on PHP 7.
On Tuesday, 1 March 2016, Christian Lück [email protected] wrote:
In src/LibEventLoop.php
#45 (comment):@@ -298,7 +298,7 @@ private function unsubscribeStreamEvent($stream, $flag)
*/
private function createTimerCallback()
{
$this->timerCallback = function ($_, $_, $timer) {
$this->timerCallback = function ($_, $__, $timer) {
I see why we would want this change, but for the reference only: Afaict
this line is currently untested because the libevent extension does not
compile under PHP 7, right?—
Reply to this email directly or view it on GitHub
https://github.com/reactphp/event-loop/pull/45/files#r54551382.
Ondřej Mirtes
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.
Thanks for the confirmation, your reasoning makes perfect sense 👍
Changes LGTM |
LGTM |
Unfortunately, Travis still reports one error for PHP 7.. Can anybody look into why this test case fails despite working for other platforms? Thanks! |
@clue will check if I find out what is wrong there. |
Here are a few facts I found out, though I am not 100% sure how to fix it.
I'd assume that this feature is working in php5 but not yet supported in php7? |
maybe @steverhoades (e4d6c7b) can give some input on what this test should do and why it is not using |
do not merge yet, this currently fails on various combinations of php versions, I'd like to fix it after reactphp#45 is merged to have tests run against all systems.
- there is no `hhvm-nightly` anymore on travis travis-ci/travis-ci#3788 (comment) - add php 7 - `event` extension works - `libev` does not support PHP 7 (yet?) m4rw3r/php-libev#8 - `libevent` also does not support PHP 7 https://pecl.php.net/package/libevent
hhvm-nightly
anymore on travis Build breaks for hhvm-nightly travis-ci/travis-ci#3788 (comment)event
extension workslibev
does not support PHP 7 (yet?) PHP 7 support m4rw3r/php-libev#8libevent
also does not support PHP 7 https://pecl.php.net/package/libevent