Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add note about ResponseInterface::toStream() #17749

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
merged 1 commit into from
Jan 18, 2023
Merged

Add note about ResponseInterface::toStream() #17749

merged 1 commit into from
Jan 18, 2023

Conversation

henrikac
Copy link
Contributor

The docs for HttpClient mentions that Symfony\Contracts\HttpClient\ResponseInterface provides a method toStream but looking at the source code one can see that this is not the case.

This PR removes the mention of Symfony\Contracts\HttpClient\ResponseInterface::toStream from the docs.

@henrikac henrikac changed the title remove toStream from docs [HttpClient] remove ResponseInterface::toStream from docs Jan 14, 2023
@javiereguiluz
Copy link
Member

Thanks for reporting this. The method is not part of the contract interface ... but it's defined by all tpyes of HttpClient responses 🤔

See https://github.com/search?q=org%3Asymfony+tostream&type=code

@henrikac
Copy link
Contributor Author

henrikac commented Jan 15, 2023 •
edited
Loading

I see. I removed toStream from this section because I noticed that the ResponseInterface contains all the methods mentioned in this section but toStream.

Edit:
I just made a test project where I installed symfony/http-client. Typing $response->toStream(); gives no autocompletion in the editor + I get this red error-line under toStream, however, the code works just fine.

Would it be better to re-formulate the intro in this section? or maybe actually add the method to the interface?

@javiereguiluz
Copy link
Member

Let's ask @nicolas-grekas to see if he can help us decide what to do here. Thanks!

@nicolas-grekas
Copy link
Member

toStream is part of StreamableInterface. Can't we mention it instead of removing info that might be useful?

@henrikac
Copy link
Contributor Author

toStream is part of StreamableInterface. Can't we mention it instead of removing info that might be useful?

Will update the PR later today to mention this instead of removing toStream.

@carsonbot carsonbot changed the title [HttpClient] remove ResponseInterface::toStream from docs remove ResponseInterface::toStream from docs Jan 17, 2023
OskarStark
OskarStark reviewed Jan 18, 2023
View reviewed changes
http_client.rst Show resolved Hide resolved
@OskarStark OskarStark changed the title remove ResponseInterface::toStream from docs Add note about ResponseInterface::toStream() Jan 18, 2023
@OskarStark OskarStark added this to the 5.4 milestone Jan 18, 2023
@OskarStark OskarStark changed the base branch from 6.2 to 5.4 January 18, 2023 04:50
@OskarStark OskarStark requested a review from xabbuh as a code owner January 18, 2023 04:50
@OskarStark
Copy link
Contributor

OskarStark commented Jan 18, 2023 •
edited
Loading

Thank you Henrik and congratulations on your first contribution to the Symfony documentation 🎉 This is much appreciated 👏

@OskarStark OskarStark merged commit 0392085 into symfony:5.4 Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Status: Needs Review
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
5 participants
@henrikac @javiereguiluz @nicolas-grekas @OskarStark @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.