cleanup phpdoc and BatchResult interface #73
Conversation
| @@ -9,6 +9,6 @@ | |||
| * | |||
| * @author Márk Sági-Kazár <[email protected]> | |||
| */ | |||
| class TransferException extends \RuntimeException implements Exception | |||
| abstract class TransferException extends \RuntimeException implements Exception | |||
There was a problem hiding this comment.
i think we never need to directly instantiate this exception. or do we?
|
I think isSuccessful and isFailed are better from a domain point of view. We are curious about if the request was successful or failed after all. |
|
BatchException never neds to have a previous exception, right? it currently does not have that option, but it also would not make much sense. |
|
Agreed. Actually there are several previous exceptions which are exposed through a custom API. |
| use PhpSpec\ObjectBehavior; | ||
| use Prophecy\Argument; | ||
|
|
||
| class TransferExceptionSpec extends ObjectBehavior |
There was a problem hiding this comment.
or is there a way to spec these things without instantiating the class?
There was a problem hiding this comment.
You can add a stub, like in case of BatchRequest trait and test that.
|
this looks ready from my point, please merge if you agree. i added php-http/utils#12 to adjust the BatchResult implementation. |
|
Do you want to add tests for TransferException prior to merging? |
|
added the TransferExcpetion spec back. ready to merge i hope? |
|
Go ahead once tests are finished. |
cleanup phpdoc and BatchResult interface
mainly adjusting the exceptions, but i also went over the BatchResult interface. we had duplicated methods and i reordered them to be consistent.
isSuccessful and hasResponseFor are the same
as are isFailed and hasExceptionFor
i went with isSuccessful and isFailed, but maybe hasExceptionFor / hasResponseFor would be more straightforward. wdyt?