Skip to content
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

Pthreads adapter & refactors #45

Closed
wants to merge 10 commits into from
Closed

Pthreads adapter & refactors #45

wants to merge 10 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 22, 2018

This PR refactors the unit tests to be functional for the adapters, instead of mocking them, to test their implementation - not the expected stacktrace. This PR also changes some other things, such as:

  • Tests for dependencies have been removed
  • StreamFactory moved from Adapters to File class
  • new FulfilledPromise -> React\Promise\resolve
  • new RejectedPromise -> React\Promise\reject
  • PHP5 support dropped
  • Bumped phpunit dep
  • Travis will not use composer.lock (for obvious reasons, such as you can't use some old versions on latest stable PHP)
  • No failures allowed
  • Some getters added to AdapterInterface (counterpart to the setters)
  • Useless classes and/or traits removed and/or merged into the class(es)
  • The child process adapter will not use count anymore for the file descriptor - instead one will be generated using PHP7's random bytes API
  • ls has been made to use a promise, instead of a stream (made compatible to the interface)
  • The ambiguous variable names of symlink have been renamed to be not ambiguous anymore
  • The type detectors will be used as necessary and only fallback to the other when necessary (to remove unnecessary iteration)
  • Adds method defined in class to filesystem interface
  • Remove opinionated usage of a specific adapter in stream
  • Pthreads adapter implemented using CharlotteDunois/Phoebe for pthreads pool with messaging and event looped worker

The PR commits will be squashed once review is positive. This PR includes and closes #46.

@ghost ghost changed the title [WIP] [WIP] Pthreads adapter & refactors Sep 22, 2018
@WyriHaximus WyriHaximus added this to the v0.2.0 milestone Sep 22, 2018
@ghost ghost changed the title [WIP] Pthreads adapter & refactors Pthreads adapter & refactors Sep 29, 2018
@WyriHaximus WyriHaximus requested review from clue, WyriHaximus and jsor and removed request for clue October 17, 2018 21:39
@WyriHaximus
Copy link
Member

Right so there is a lot of good stuff in this PR and I'm really happy with that but it is basically to big to review in its current state. So I would prefer we break this up in separate PR's. Preferable starting with Remove opinionated usage of a specific adapter in stream so we can release that as part of v0.1.3, before moving on to all the breaking changes for v0.2.

Now I appreciate the huge effort you put into all the fixes and the new pthreads adapter and I know that asking to split this up isn't going to speed this up initially but it will allows us to iterate over smaller PR's faster then a single huge one.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework file stream API
1 participant