-
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
[1.0][resolver] Decouple WebPathResolver from http request. Simplify its logic. #320
[1.0][resolver] Decouple WebPathResolver from http request. Simplify its logic. #320
Conversation
@havvg ping |
@@ -23,7 +23,7 @@ function isStored($path, $filter); | |||
* @param string $path The path where the original file is expected to be. | |||
* @param string $filter The name of the imagine filter in effect. | |||
* | |||
* @return string The URL of the cached image. | |||
* @return string The ABSOLUTE URL of the cached image. |
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.
Don't yell :-)
The new name is misleading. The resolver has nothing to do with the web path (even if the previous implementation extended the web path resolver). It's more a |
disagree, it expects that given |
@havvg fix ABSOLUTE and test name, could we address provider maybe later? |
Yeah, let's postpone this one for now :) |
…e-from-request [1.0][resolver] Decouple WebPathResolver from http request. Simplify its logic.
thanks! we are moving forward fast 👯 |
This PR introduce next features:
WebPathResolver
andNoCacheWebPathResolver
no more coupled to request. Could be executed in cli. You just have to configure parameters used by request context as default values.AbstractFilesystemResolver
was removed.NoCacheResolver
was renamed toNoCacheWebPathResolver
since could only work with local images. I think previous name would confuse someone.WebPathResolverTest
are units and does not use real filesystem any more.WebPathResolver
relly on filesystem services everywhere.CreateCacheDirectoriesCompilerPass
was removed as it seems filesystem does its job very well (I tested it in different scenarios).