Skip to content

Support both evenement 3.0 and 2.0 (and react/stream) #22

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
Merged
WyriHaximus merged 2 commits into from
Mar 24, 2018
Merged

Support both evenement 3.0 and 2.0 (and react/stream) #22

WyriHaximus merged 2 commits into from
Mar 24, 2018

Conversation

@WyriHaximus
Copy link
Member

@WyriHaximus WyriHaximus commented Feb 28, 2018 •
edited
Loading

Événement 3.0 is nearly fully backwards compatible with 2.0 and react/filesystem is fully compatible with all three so why not support it. It packs some neat performance upgrades without any code changes on react/filesystem's side :shipit: .

@WyriHaximus WyriHaximus added this to the v0.1.1 milestone Feb 28, 2018
@WyriHaximus WyriHaximus requested review from clue and jsor February 28, 2018 21:17
@WyriHaximus WyriHaximus mentioned this pull request Feb 28, 2018
Closed
clue
clue reviewed Mar 1, 2018
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.

This currently doesn't have any effect because the Stream version still locks this to an older version. Arguably, this basically means that this is PR is currently untested :-) Does it make sense to update the Stream component as part of this PR? 👍

@WyriHaximus
Copy link
Member Author

WyriHaximus commented Mar 1, 2018

@clue good point, I'll look into that after I fixed the current reason builds fail

@ghost
Copy link

ghost commented Mar 1, 2018 •
edited by ghost
Loading

I have seen that wyrihaximus/react-child-process-messenger locks reactphp/stream down to max. v0.5 by requiring reactphp/child-process v0.4. There seems to be a PR for this, but there hasn't been any recent activity.

@WyriHaximus
Copy link
Member Author

WyriHaximus commented Mar 1, 2018

@CharlotteDunois you mean this PR WyriHaximus/reactphp-child-process-messenger#13 ? There are some things locally I want to push and finish before merging it, once tagged the only thing in the chain requiring stream 0.4 is this package it self.

@WyriHaximus
Copy link
Member Author

WyriHaximus commented Mar 23, 2018

@clue done 👍

@WyriHaximus WyriHaximus changed the title Support both evenement 3.0 and 2.0 Support both evenement 3.0 and 2.0 (and react/stream) Mar 23, 2018
clue
clue approved these changes Mar 24, 2018
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, thanks for the update, changes LGTM! :shipit:

@WyriHaximus
Copy link
Member Author

WyriHaximus commented Mar 24, 2018

Follow up with fixes for failing tests are coming soon as well 👍

@WyriHaximus WyriHaximus merged commit 01128b7 into reactphp:master Mar 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsor jsor jsor approved these changes

@clue clue clue approved these changes

Assignees

No one assigned

Labels

easy pick help wanted maintenance

Projects

None yet

Milestone

v0.1.1

Development

Successfully merging this pull request may close these issues.

3 participants

@WyriHaximus @jsor @clue