reactphp / filesystem

Evented filesystem access.
MIT License
135 stars 40 forks source link

Allow PHP 5.4 to fail #31

Closed WyriHaximus closed 6 years ago

WyriHaximus commented 6 years ago

PHPUnit on PHP 5.4 seems to hang at the end of it's execution for no apparent reason. (Even when successful.)

WyriHaximus commented 6 years ago

Dropping 5.4 support would make more sense for 0.2.0 tbh, or even better bump it to 5.6/7.0. I'm fine with postponing until the rest is in for now.

ghost commented 6 years ago

The supported version should be bumped to PHP7, as the official support for PHP5 is running out this year (31 Dec 2018), so it doesn't make much sense to support PHP5 for long.

Plus you won't use react components on webspaces (where you're stuck with old versions). :) ...unless you heavily misunderstand the concepts (which has happened before, poor me who tried to put the guy back onto the rails).

WyriHaximus commented 6 years ago

@CharlotteDunois Yup, it's one of the things I want to do for 0.2.0, a long side with a bunch of other improvements

WyriHaximus commented 6 years ago

@clue We have two possible paths here:

a) We merge this and rebase the other PR's and merge those as well, and then release 0.1.1 bringing all updated ReactPHP packages to our users. And fix the two broken builds in 0.1.2/0.2.0 which ever will be preference/chosen path.

b) Go down the rabbithole and figure out why it hangs under certain specific conditions, and delay 0.1.1 for weeks at least.

My preference goes out to a).

clue commented 6 years ago

@WyriHaximus I'm not too concerned about legacy PHP 5.4, but I wonder why the same issue seems to manifest itself with PHP 7.1 as well and not any other tested PHP version? The test output suggests that the tests run just fine and yet the PHP engine is somehow left in a "stuck" state and won't exit?

That being said, this PR makes the test suite pass on all other versions at least (finally green again!). As such, I would rather not block this PR and leave this decision up to you to fix this at a later time :shipit:

WyriHaximus commented 6 years ago

@WyriHaximus I'm not too concerned about legacy PHP 5.4, but I wonder why the same issue seems to manifest itself with PHP 7.1 as well and not any other tested PHP version? The test output suggests that the tests run just fine and yet the PHP engine is somehow left in a "stuck" state and won't exit?

Same here, and I would prefer dropping 5.x in 0.2. Not sure why but I'm going to look into that once 0.1.1 is out.

That being said, this PR makes the test suite pass on all other versions at least (finally green again!). As such, I would rather not block this PR and leave this decision up to you to fix this at a later time :shipit:

Cheers :shipit: !