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

Remove public $stream property from all resource streams #90

Merged
merged 1 commit into from Apr 27, 2017

Conversation

Projects
None yet
3 participants
@clue
Member

clue commented Apr 27, 2017

Remove public $stream property from all resource streams. This property has attracted some low quality code and interacting with it could easily break the stream semantics.

Empirical evidence suggests this isn't really needed anyway, so I propose to remove it from the public API for now. Should we need this again in the future, it's a simple feature addition with no BC break, so I'd suggest removing this until we find a use case that actually relies on this.

@clue clue added the BC break label Apr 27, 2017

@clue clue added this to the v0.7.0 milestone Apr 27, 2017

@WyriHaximus

This comment has been minimized.

Member

WyriHaximus commented Apr 27, 2017

Any idea why the tests are failing?

@clue clue force-pushed the clue-labs:stream-property branch from fc3dcde to e99b556 Apr 27, 2017

@clue

This comment has been minimized.

Member

clue commented Apr 27, 2017

Not sure what's going on with Travis at the moment, the build logs are not available currently, but force-pushing this again seems to have fixed the failing tests :shipit:

@jsor

jsor approved these changes Apr 27, 2017

@clue clue merged commit 585af73 into reactphp:master Apr 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@clue clue deleted the clue-labs:stream-property branch Apr 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment