Skip to content
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

added reference type to generate url #1224

Merged
merged 5 commits into from Sep 16, 2019
Merged

added reference type to generate url #1224

merged 5 commits into from Sep 16, 2019

Conversation

eroluysal
Copy link
Contributor

@eroluysal eroluysal commented Sep 12, 2019

Q A
Branch? 2.0
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

I want to use cdn with imagine_filter and asset methods. But imagine_filter returns absolute url everytime and asset ignores my defined assets url.

I was added referenceType argument to generateUrl method of cache manager.

Copy link
Member

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like a reasonable idea to me. please change the default value to avoid a BC break.

Imagine/Cache/CacheManager.php Outdated Show resolved Hide resolved
Imagine/Cache/CacheManager.php Outdated Show resolved Hide resolved
@eroluysal eroluysal requested a review from dbu September 16, 2019 08:06
Copy link
Member

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@dbu dbu merged commit 8dcc8c1 into liip:master Sep 16, 2019
@ossinkine
Copy link
Contributor

@eroluysal I guess you make something like asset('path'|imagine_filter('filter')), so you need a relative path. But you can use ProxyResolver, it can override the origin domain to your CDN domain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants