-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: add OutgoingRequestInterface #6698
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
Conversation
5589339 to
d6150ff
Compare
|
I'm not sure if you implemented it this way because of the restrictions in our current HTTP layer, but the inheritance path for requests is confusing. What is restricting us from making Request a common base for both Incoming and Outgoing, and those not being in the same path? |
|
PSR-7: MessageInterface > RequestInterface (outgoing) > ServerRequestInterface (incomming)
CI4's Request is incomming request. |
|
Wow you are totally right! TIL an incoming request is actually an outgoing request with some extra methods. So next question... why the new class? If we already have Message > Request > IncomingRequest, what is stopping us from making Request into the equivalent of OutgoingRequest? (sorry if this is obvious, it is very hard to flip between interfaces on mobile) |
The reason for adding the new class is to make as few changes as possible.
Do you mean like this? |
|
I'm with you now, took me a little bit. The structure and names are confusing, but that's not your fault. I would like to look at this one more time on desktop, but generally I think it is a good direction and you should feel free to proceed with the docs. |
2404804 to
eda2615
Compare
|
Added changelog. |
MGatner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're there! I would like one other set of eyes here. @paulbalandan or @samsonasik or @iRedds ?
eda2615 to
d58f681
Compare
d58f681 to
583689c
Compare
Description
See #6573, #4356
OutgoingRequestOutgoingRequestInterfaceCURLRequestextendsOutgoingRequestRequestextendsOutgoingRequestRequestInterfaceextendsOutgoingRequestInterfacehttp://www.plantuml.com/plantuml/uml/ZP1TQWCX58NVNSKX2-XJ5n38IuMKbf0kGEoTGUhFLAzGADrxnQOIbCpaRPXpplUf-yGgSdPMB4f_g9cmEyZ77Ru501ZF52Ub2S-KKadb_uykViay1-Fd4trcIjnge2yc_vwszhTsDy6Y0hHLgR5Xt6B9aUTHrze3iPb6oYWVQJsbcrpJrguWlGvkE5JRrAOFJE2m84nzl-Q0yZ2N0F52caB4q_dy-b0x1Levr-x3KTwhRTKjOock2DzEnurtwyrrCwZ3twYfE-MFs9VlxEml
Checklist: