-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Added note about async non-goal #942
Conversation
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.
thanks. have one suggestion but am ok to merge as is.
|
||
A separate interface for asynchronous requests can be defined in a separate PSR | ||
once the promise PSR is accepted. The method signature for asynchronous requests | ||
has to be different. |
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.
maybe:
...has to be different from the method signature for synchronous requests because the return type has to be different anyways (Promise vs Response).
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.
Great. Thanks1
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.
thank you!
#### Asynchronous HTTP client | ||
|
||
The reason asynchronous support is a non-goal is because a PSR for HTTP clients | ||
should not define Promises. That should be a separate PSR. Waiting for a such PSR |
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.
Thinking about a wait part: it only really matters while the PSR is WIP, but afterwards it's not an information. The fact that it should be a separate PSR and at the time of writing there was no such PSR should be enough to be mentioned IMO.
@sagikazarmark, better now? |
|
||
The reason asynchronous support is a non-goal is because a PSR for HTTP clients | ||
should not define Promises. That should be a separate PSR. At the time the HTTP | ||
client PSR was written there where no PSR for Promises. |
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.
there was no such PSR
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.
oops, was way to quick there. Thanks
Definitely. Just one last small thing. |
A separate interface for asynchronous requests can be defined in a separate PSR | ||
once the promise PSR is accepted. The method signature for asynchronous requests | ||
has to be different from the method signature for synchronous requests because | ||
the return types of asynchronous calls will be a Promise. |
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.
"the return type of asynchronous calls" (return type should be singular here, not plural)
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.
thank you!
This will replace php-http#31