[WIP] Libuv Adapter #69
Conversation
There was a problem hiding this comment.
@CharlotteDunois Love the direction, thanks for working on this! 🥇 👍
What is missing in this PR? Is there anything that we can do to help this progress?
More of an aside, but personally I would probably vote to not depend on #67 at the moment, as I don't see any major dependency here.
|
@clue Unit tests are added for libuv (which pass), however I noticed there are some underlying issues with the libuv event loop, because other unit tests are failing (with the libuv event loop and eio or child process adapter). This requires more investigation. And the major dependency on #67 for this PR is performance and future security fixes. I don't like supporting old, unmaintained versions, simply because it encourages others to not upgrade (mostly comfort and laziness in their case). ReactPHP (and mostly the event loop and filesystem components) are targeting CLI applications anyway, which requires access through SSH (or similar mechanisms) to work. If you have SSH access you can simply upgrade the PHP version for any Linux distribution in a very simple way (for Windows you can just download a new binary). And let's be honest here, there's no reason not to upgrade to PHP7. Dropping support for PHP5 encourages others to upgrade. It's just my opinion, but I see no reason why we should keep supporting PHP5. Bugfixes have stopped over two years ago. |
Interesting, let's focus on this part to move this forward. Does it make sense to address this in a separate ticket? Does anybody have any insights? |
Maybe it makes sense to address this in a separate ticket. However I see no value gained if merging this PR causes everything else to break. If you're interested in a travis build history, here's one, they're mostly timeouts. Some tests work with ext-uv and child process adapter and some don't - why is unknown to me. |
ghost
closed this
This PR adds a libuv based adapter to
react/filesystem. This PR builds on top of #65 and #67 and as such stays as draft until these PRs get merged. The PR will be rebased once ready.For the UV adapter the unit tests are functional and will build a foundation for a future pull request to refactor the unit tests. I've added a helper method to concat paths for cross compatibility.
This PR is not fully ready yet, but opening a PR will help getting some comments and feedback during development. This PR closes #9.