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

add default shopware cdn strategy in .env #138

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Conversation

zaifastafa
Copy link
Contributor

No description provided.

@shyim
Copy link
Owner

shyim commented Jan 4, 2022

why is this needed?

@zaifastafa
Copy link
Contributor Author

why is this needed?

I have seen several shops that we clone for local development has this CDN strategy in their .env file but our setup does not create this variable when setting up a shop, so thought it'd be good for consistency.

@shyim
Copy link
Owner

shyim commented Jan 4, 2022

The argument works also in other way 😅

Maybe define in that projects this explict in the config/services.xml

@zaifastafa
Copy link
Contributor Author

So this is not needed in your opinion? Without this variable, the media images show broken in the shop if we have done a clone.

@zaifastafa
Copy link
Contributor Author

I have verified it that when I remove this variable, the media images are broken. But when I add it, it is able to identify the path of the images correctly.

@shyim
Copy link
Owner

shyim commented Jan 4, 2022

I guess the production system have a configuration like this in the config/packages https://github.com/shopware/production/blob/6.4/config/services/defaults.xml#L19-L20

@zaifastafa
Copy link
Contributor Author

Here it defaults to physical_filename, but I saw in a commit I think that it was defaulting to id in the yaml file, so I suppose we can use the default cdn strategy to be id here too? That is easier to maintain I think then changing the yaml file for local setups.

@shyim shyim merged commit 2ed73d5 into shyim:v1 Jan 6, 2022
@zaifastafa zaifastafa deleted the patch-1 branch January 6, 2022 11:36
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

2 participants