Add exception explanation, fixes #87 #93
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
looks good to me. for async, we could mention in the text part that exceptions that should be handled like network issues and server problems are available from the promise, as they might not be known yet when the sendAsyncRequest method returns. |
|
Better now? (Should be squashed) |
sagikazarmark
force-pushed
the
exceptions
branch
2 times, most recently
from
967030b to
d7ea978
Compare
|
looks good to me, yes! |
|
@joelwurtz ping |
sagikazarmark
force-pushed
the
exceptions
branch
2 times, most recently
from
fcb112b to
f7508ed
Compare
| @@ -21,7 +21,8 @@ | |||
| * | |||
| * @return ResponseInterface | |||
| * | |||
| * @throws Exception | |||
| * @throws Exception If an error happens during processing the request. | |||
| * @throws \Exception If processing the request is impossible (eg. bad configuration). | |||
There was a problem hiding this comment.
Can we use this exception naming instead:
@throws \Http\Exception
@throws \ExceptionCurrently it's feel a bit weird and i'm afraid of misunderstanding from user, if it's read like that
There was a problem hiding this comment.
agreed for this situation. but in general we should keep using the use statement for phpdoc as well.
|
Aside from my comment, 👍 for me |
sagikazarmark
force-pushed
the
exceptions
branch
2 times, most recently
from
82217bd to
85f3c52
Compare
Add missing trailing dot Add better explanation for workflow exceptions in Async Client Use FQCN to avoid confusion
sagikazarmark
force-pushed
the
exceptions
branch
from
85f3c52 to
dfbf75b
Compare
sagikazarmark
added a commit
that referenced
this pull request
Dec 15, 2015
Add exception explanation, fixes #87
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@dbu @joelwurtz please help me with the explanation. 😄