-
Notifications
You must be signed in to change notification settings - Fork 57
Throw exception if response is already completed #30
Throw exception if response is already completed #30
Conversation
codeliner
commented
Sep 10, 2015
- Closes PSR-7 violations #24
@weierophinney can you let me know if this PR is legit? I'm not experienced with the API, so I don't know if we're fixing broken API or actually changing it. |
@Ocramius This change was discussed with @weierophinney in #24 |
Next minor release; we're still in 0.* :-)
|
@weierophinney v1.1.0 doesn't look like a dev release ;) Confused with zend-expressive? Or did I miss something? |
@codeliner Yep. That's what I get by answering notifications via email. |
;) |
return new RuntimeException(sprintf( | ||
'Calling %s::%s is not possible. Response is already completed', | ||
__CLASS__, | ||
$detectedInMethod |
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.
__METHOD__
includes both the method name and the class name; as such, you can simplify %s::%s
to just %s
, and remove the __CLASS__
argument.
As noted in #24, and my yourself, this is a BC break, but also fixes bad behavior. As such, I'm going to schedule this for the next minor release, where we can call it out in the release notes. |
Throw exception if response is already completed
Updated the exception thrown from `responseIsAlreadyCompleted` to remove the `%s::%s` notation and the `__CLASS__` argument, as the method passed is `__METHOD__`. Additionally, fixed the tests to run under PHP 5.4 (as we still support that version).