Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Used relative stream to simplify emitBodyRange. Removes necessity to check lower bound #202

Merged

Conversation

hannesvdvreken
Copy link
Contributor

Replaces #188

$body->seek($first);
$pos = $first;
$body = new RelativeStream($response->getBody(), $first);
$body->rewind();
Copy link
Member

Choose a reason for hiding this comment

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

This directly conflicts with #200 (represented in lines 84 - 88 above), which provided a fix that ensures that the emitter can be used with non-seekable streams. Please reinstate that block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I saw that. Will find some time to fix that asap.

@hannesvdvreken
Copy link
Contributor Author

@weierophinney should be fixed now 😉 waiting for the build.

@weierophinney
Copy link
Member

Looks like the rebase dropped the lines entirely around testing for seekable streams, @hannesvdvreken — please see: https://travis-ci.org/zendframework/zend-diactoros/jobs/158205643#L240-L248

@weierophinney
Copy link
Member

@hannesvdvreken I can see how to fix this; I'll take it from here.

@weierophinney weierophinney self-assigned this Sep 7, 2016
@weierophinney weierophinney merged commit 5e38627 into zendframework:master Sep 7, 2016
weierophinney added a commit that referenced this pull request Sep 7, 2016
Used relative stream to simplify emitBodyRange. Removes necessity to check lower bound
weierophinney added a commit that referenced this pull request Sep 7, 2016
weierophinney added a commit that referenced this pull request Sep 7, 2016
@weierophinney
Copy link
Member

Thanks, @hannesvdvreken

@hannesvdvreken
Copy link
Contributor Author

I was away from computer for a while. Thanks for finishing this, @weierophinney.

@weierophinney
Copy link
Member

My pleasure, @hannesvdvreken — thanks for the patch!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants