-
Notifications
You must be signed in to change notification settings - Fork 379
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
Fixes #373. Replace NotFoundHttpException with SourceNotFoundException #403
Fixes #373. Replace NotFoundHttpException with SourceNotFoundException #403
Conversation
|
||
use Liip\ImagineBundle\Exception\ExceptionInterface; | ||
|
||
class SourceNotFoundException extends \RuntimeException implements ExceptionInterface |
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.
Could you name it NotLoadableException
so we consistent with ones we already have https://github.com/liip/LiipImagineBundle/tree/master/Exception/Imagine/Cache/Resolver
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.
I dont like the name but at least it consistent
@Me1ifaro while you on it could you check other loaders too. I am sure |
|
@makasim, seems that I merged master instead of rebasing from it... Can you get me the point how to fix this? |
@makasim, ok seems like rebasing from upstream/master solved the problem |
@Me1ifaro looks good, could you look at other loaders? |
@makasim, I'm sure that I made project wide search :) |
you can always cherry pick desired commits by their hash |
@Me1ifaro GridFsLoader is not present in the diffs and Doctrine abstract loader too. |
Seems like I made some magic with git |
@makasim, anything else should be checked? |
…xceptions Fixes #373. Replace NotFoundHttpException with SourceNotFoundException
@Me1ifaro thanks |
this PR breaked the test-suite: https://travis-ci.org/liip/LiipImagineBundle/builds/23912174 @Me1ifaro could you refix this? |
@digitalkaoz, sure. |
@digitalkaoz, done. See #419 |
Fixes #373