PeclEvLoop: Add new PeclEvLoop (PECL ext-ev)#97
Uh oh!
There was an error while loading. Please reload this page.
Conversation
The existing libev driver seems to function only with a non-official, out of date libev extension for PHP. This PeclEvLoop class is a very similar clone of the LibEvLoop library but for the officially documented libev extension. Changes are mostly to constants and function parameter order.
ivkalita commented Apr 8, 2017
Sorry, I forgot to add travis configuration for PECL ev extension (which is necessary for |
WyriHaximus commented Apr 8, 2017
@kaduev13 tbh no need to close your PR for that, just add another commit fixing that. Or just squash it in your last commit. (@clue is our resident Git guru) |
@WyriHaximus ok, thank you. I'm very new to open-source, but I will learn fast ;) Btw, it's fixed now. |
src/PeclEvLoop.php Outdated
| /** | ||
| *{@inheritdoc} | ||
| */ | ||
| public function tick() |
There was a problem hiding this comment.
Note, that tick() has been removed from the interface (see #72).
| /** | ||
| *{@inheritdoc} | ||
| */ | ||
| public function addReadStream($stream, callable $listener) |
There was a problem hiding this comment.
The docblocks containing just {@inheritdoc} can be removed.
There was a problem hiding this comment.
@jsor, no problem, I can remove it, but {@inheritdoc} makes inheritance explicit (link to PSR-5 inheritance section). Is it a reactphp project convention?
There was a problem hiding this comment.
Since this method implements the interface, it's explicit that the docblock is inherited. We tend to avoid docblocks since they are often redundant and become easily out of date. With type hints and return types in PHP 7, they will become even more obsolete (at least 99.9% of the time) :)
There was a problem hiding this comment.
@jsor, ok, thank you for the explanation 😃 , I agree with your arguments (but I just looked through LibEvLoop, StreamSelectLoop and other loops sources and there are a lot of "only @inheritdoc" PHPDocs).
I removed unnecessary @inheritdoc blocks from PeclEvLoop in 7128aaf.
Btw, I think that it'll be better to cleanup the history of this PR before merging it into master (cause of a lot of little fix commits). I will do it tomorrow morning (UTC).
| /** | ||
| * @author Ivan Kalita kaduev13@gmail.com | ||
| */ | ||
There was a problem hiding this comment.
We usually don't add @author doc blocks.
jsor commented Apr 25, 2017
ping @WyriHaximus and @clue |
WyriHaximus commented May 24, 2017
ping @clue |
WyriHaximus commented Jul 26, 2017
| use SplObjectStorage; | ||
| /** | ||
| * @see https://bitbucket.org/osmanov/pecl-ev/overview |
| * | ||
| * @return \Closure | ||
| */ | ||
| private function getStreamListenerClosure($stream, callable $listener){ |
There was a problem hiding this comment.
Minor nitpick: Line break before { (PSR-2)
ivkalita commented Jul 26, 2017
@clue Thank you for the corrections, I'll fix these issues on this weekend 😃 |
WyriHaximus commented Aug 23, 2017
@kaduev13 any luck with those corrections? |
clue changed the title 
clue commented Feb 7, 2018
Superseded by #148 👍 |
5 participants
Improved and (just a little bit) refactored #25.