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.

Sign up for GitHub

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

Jump to bottom

UV Event Loop (PECL ext-uv) #112

Merged
merged 1 commit into from
Jan 13, 2019
Merged

UV Event Loop (PECL ext-uv) #112

merged 1 commit into from
Jan 13, 2019

Conversation

WyriHaximus
Copy link
Member

No description provided.

@WyriHaximus WyriHaximus added this to the v0.5.2 milestone Oct 31, 2017
@WyriHaximus WyriHaximus requested review from jsor, cboden and clue October 31, 2017 10:48
@WyriHaximus WyriHaximus force-pushed the blacklight branch 4 times, most recently from c6dcbde to 7a359c5 Compare October 31, 2017 16:39
kelunik
kelunik reviewed Oct 31, 2017
View reviewed changes
kelunik
kelunik reviewed Oct 31, 2017
View reviewed changes
@clue clue mentioned this pull request Nov 7, 2017
Closed
clue
clue reviewed Nov 8, 2017
View reviewed changes
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, would love to see more extensions find their way into this project! 👍

@clue clue changed the title UV Event Loop UV Event Loop (unofficial ext-libuv) Dec 4, 2017
@clue clue mentioned this pull request Dec 4, 2017
Closed
@WyriHaximus WyriHaximus mentioned this pull request Feb 23, 2018
Closed
kelunik
kelunik reviewed Feb 23, 2018
View reviewed changes
@WyriHaximus WyriHaximus mentioned this pull request Feb 25, 2018
Closed
@WyriHaximus WyriHaximus requested a review from clue April 9, 2018 15:56
@WyriHaximus
Copy link
Member Author

@clue @jsor right, rebased this PR against the latest master. Could you review?

clue
clue requested changes Apr 24, 2018
View reviewed changes
@clue clue modified the milestones: v0.5.2, v0.5.3 Apr 24, 2018
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Apr 24, 2018
@WyriHaximus
Renamed LibUvLoop to ExtUvLoop after feedback from @clue and @kelunik …
b423bfe 
…at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Apr 24, 2018
@WyriHaximus
Aligned ext-uv detection method to check for the uv_loop_new function…
10545d9 
… as suggested by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Aligned ext-uv detection method to check for the uv_loop_new function…
f20dde0 
… as suggested by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Drop PHP 7 version check as the ext-uv loop only works on PHP 7 anywa…
ad0d2f7 
…y as suggested by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Ensure intervals passed nto uv_timer_start are integers and rounded…
e9881a4 
… off upwards as suggested by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Use different arrays for read and write listeners as suggested by @clue…
8db0f57 
… at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Took into account if the loop is still running and if there are any s…
2655e98 
…ignal listeners attached for calling `uv_run`, modeled (copied) the bit from `ExtEvLoop` and had to rename $timerEvents to $timers (also makes more sense) as suggested by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Reverted a test timer change, can't remember of find why this was cha…
e5209e0 
…nged and what failed before the change, as suggested by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Missing timer tests as pointed out by @clue at reactphp#112 (comment)
95b7634
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Install uv from PECL via @clue at reactphp#112 (comment)
f4d8132
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Updated readme and uv loop dockblock with it's release on PECL via @clue
94f4d7b 
 at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Removed the flags property as pointed out by @clue at reactphp#112 (c…
b390839 
…omment)

The property was only used in one method and adding more bloat than solving a problem
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Dec 10, 2018
@WyriHaximus
Removed PHP 7 version check as ext-uv is only available for PHP 7 via @…
2aed93b 
…clue at reactphp#112 (comment)
@WyriHaximus
Copy link
Member Author

@clue && @jsor && @cboden could you review this PR again. I feel it's ready to be merged and released into the wild 🎉 .

@WyriHaximus
Copy link
Member Author

FYI the issues I was having wheren't related to the event loop directly but due to file descriptors being passed on child processed by default, solving that down/upstream depending on your perspective: WyriHaximus/reactphp-child-process-messenger#22

clue
clue reviewed Jan 11, 2019
View reviewed changes
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WyriHaximus added minor remarks only, can you squash this to a reasonable number of commits before the next review? :shipit:

src/ExtUvLoop.php Outdated
if ($status !== 0) {
unset($this->flags[(int) $stream]);
$this->pollStream($stream);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @WyriHaximus, what's the status here?

WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Jan 11, 2019
@WyriHaximus
Removed left over bug demonstration using travis, pointed out by @clue …
f517a17 
…at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Jan 11, 2019
@WyriHaximus
Ensure the new loop follows the standard set by @Ocramius in reactphp…
0a67e3f 
…#167, pointed out by @clue at reactphp#112 (comment)
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Jan 11, 2019
@WyriHaximus
Ssuperfluous space, pointed out by @clue at reactphp#112 (comment)
b098273
@WyriHaximus
Copy link
Member Author

@clue will squash into one commit once everyone approves it before merging it

ghost
ghost suggested changes Jan 11, 2019
View reviewed changes
WyriHaximus added a commit to WyriHaximus-labs/event-loop that referenced this pull request Jan 11, 2019
@WyriHaximus
Added missing space after type cast thanks to @CharlotteDunois at rea…
1e951a3 
…ctphp#112 (comment) and reactphp#112 (comment)
clue
clue approved these changes Jan 13, 2019
View reviewed changes
Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's happening: Squash and then let's get this in! :shipit: 🎉

@WyriHaximus
Copy link
Member Author

Ping @jsor && @cboden

@WyriHaximus
Copy link
Member Author

It is time for this PR to be merged, I've squashed all commits into one, the original unrewritten history can be found here: https://github.com/WyriHaximus-labs/event-loop/tree/blacklight-history

@WyriHaximus WyriHaximus removed the request for review from cboden January 13, 2019 20:33
@WyriHaximus WyriHaximus merged commit 1ed8f4b into reactphp:master Jan 13, 2019
@WyriHaximus
Copy link
Member Author

webp net-gifmaker 1

@WyriHaximus WyriHaximus mentioned this pull request Feb 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@docteurklein docteurklein docteurklein left review comments

@kelunik kelunik kelunik left review comments

@jsor jsor jsor approved these changes

@clue clue clue approved these changes

Assignees
No one assigned
Labels
new feature
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.
6 participants
@WyriHaximus @andreybolonin @jsor @docteurklein @clue @kelunik