Add back NotFoundException for BC #69
Conversation
|
|
||
| namespace Http\Discovery; | ||
|
|
||
| @trigger_error('The '.__NAMESPACE__.'\NotFoundException class is deprecated since version 1.0 and will be removed in 2.0. Use Http\Discovery\Exception\NotFoundException instead.', E_USER_DEPRECATED); |
There was a problem hiding this comment.
Do we need the NAMESPACE constant here?
There was a problem hiding this comment.
i think as long as we ourselves throw the old exception, we can't trigger a warning.
RE: namespace or not: does not matter imo. writing the actual namespace seems more straightforward though
There was a problem hiding this comment.
I took this namespace thing from symfony. Since the namespace declaration is just above the warning, I think we are allowed to make it shorter in the code. The message itself is what's important after all.
But you are right, maybe we shouldn't throw this as long as we actually throw this exception.
|
👍 Good, I had just one minor question. |
| * @author Márk Sági-Kazár <[email protected]> | ||
| */ | ||
| final class NotFoundException extends \RuntimeException | ||
| /*final */class NotFoundException extends \RuntimeException |
There was a problem hiding this comment.
maybe add "(do not extend this class, it is only not final for BC reasons)"
There was a problem hiding this comment.
Isn't this what the @final phpdoc is for?
There was a problem hiding this comment.
Isn't this what the @final phpdoc is for?
There was a problem hiding this comment.
ah, makes sense. maybe add that comment after the @final then?i think explanations always are a good thing.
|
looks good to me. squash and merge? |
|
Yup, but rebase first. |
sagikazarmark
force-pushed
the
exception_bc
branch
from
90c48be to
b79c5e4
Compare
As per the discussion here: #62 (comment)