Skip to content

Forward compatibility with Stream v0.5 - #26

Merged
WyriHaximus merged 2 commits into
reactphp:masterfrom
clue-labs:stream
Mar 10, 2017
Merged

Forward compatibility with Stream v0.5#26
WyriHaximus merged 2 commits into
reactphp:masterfrom
clue-labs:stream

Conversation

@clue

@clueclue commented Mar 8, 2017

Copy link
Copy Markdown
Member

With some minimal changes, this component is in compatible with v0.4 and the new v0.5.

jsor
jsor approved these changes Mar 8, 2017
@clue

clue commented Mar 8, 2017

Copy link
Copy Markdown
MemberAuthor

Updated to include documentation about STDIO streams, so this now closes #17.

Comment threadREADME.md
and you can use any of its events and methods as usual:

```php
$process->stdout->on('data', function ($chunk) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

$chunk -> $data

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I'd rather leave this like this in order to emphasize this is not necessarily a complete "data message", but could possibly be only a small "data chunk". This is also in line with all other stream documentations.

Any objections / recommendations?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't care, but you need to change the line below to echo $chunk; :)
Sorry, for not being clear enough.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Duh, thanks for spotting, fixed and squashed :shipit:

jsor
jsor approved these changes Mar 9, 2017
@clue

clue commented Mar 10, 2017

Copy link
Copy Markdown
MemberAuthor

Rebased now that #27 is in :shipit:

@WyriHaximus
WyriHaximus merged commit 797a920 into reactphp:masterMar 10, 2017
@clue
clue deleted the stream branch March 10, 2017 12:55
Sign up for freeto 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.

3 participants

@clue@jsor@WyriHaximus