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

Merge HttpClient component into this component #148

Closed
clue opened this issue Mar 19, 2017 · 2 comments · Fixed by #368
Closed

Merge HttpClient component into this component #148

clue opened this issue Mar 19, 2017 · 2 comments · Fixed by #368

Comments

@clue
Copy link
Member

clue commented Mar 19, 2017

Here's the current situation:

  • This Http component: Event-driven, streaming plaintext HTTP and secure HTTPS server for ReactPHP.
  • HttpClient component: Asynchronous HTTP client library.

This is not exactly ideal, they share quite a bit of common code.

Also, I believe this situation may be a bit confusing for consumers of this package. In particular our Datagram component provides both client and server side for datagram sockets (UDP).

As such, I'd vote for merging these two components into a single Http component.

@cebe
Copy link

cebe commented Jan 15, 2018

they share quite a bit of common code.

Could you point out which code they share? When looking at the code I could not find anything obvious.

@clue
Copy link
Member Author

clue commented Jan 16, 2018

Could you point out which code they share? When looking at the code I could not find anything obvious.

Good point. You're right in that they currently don't share much code (except for chunked transfer encoding maybe). However, they share quite a bit of domain knowledge. In particular, we've had to backport fixes / minor features from this component to the http-client repeatedly.

Also, looking at its roadmap (reactphp/http-client#79) reveals that we plan to implement PSR-7 interfaces (reactphp/http-client#41) also for the http-client. Once this is in, they will share significantly more code.

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

Successfully merging a pull request may close this issue.

2 participants