Skip to content
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

Documentation to recommend Server by default instead of advanced StreamingServer #284

Merged
merged 2 commits into from
Dec 12, 2017

Conversation

clue
Copy link
Member

@clue clue commented Dec 12, 2017

This PR adds some documentation to recommend Server by default instead of the advanced StreamingServer. I've tried to keep changes to a minimum to ease review.

There's still potential for a follow-up PR to eventually improve the README structure 👍

Builds on top of #266 and #271

Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

README.md Outdated
@@ -160,10 +200,11 @@ Check out [request](#request) for more details.

### Request

An seen above, the `StreamingServer` class is responsible for handling incoming
connections and then processing each incoming HTTP request.
An seen above, the [`Server`](#server) and [`StreamingServer`](#streamingserver)
Copy link
Member

Choose a reason for hiding this comment

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

An -> As

README.md Outdated
been received (see also [`RequestBodyBufferMiddleware`](#requestbodybuffermiddleware)
for an alternative).
If you're using the advanced [`StreamingServer`](#streamingserver), the
request object will be procesed once the request headers have been received.
Copy link
Member

Choose a reason for hiding this comment

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

procesed -> processed

Copy link
Member

@jsor jsor left a comment

Choose a reason for hiding this comment

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

2 minor typos, otherwise 👍 :shipit:

@clue
Copy link
Member Author

clue commented Dec 12, 2017

@jsor Good catch, updated to fix minor typos :shipit:

@jsor jsor merged commit 0f0d293 into reactphp:master Dec 12, 2017
@clue clue deleted the server-streaming branch December 12, 2017 12:00
Sign up for free to 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